WASM compatibility: drop dart:io from import graph (pana 160/160)#95
Merged
Conversation
Replace dart:io Platform.is* with Flutter's WASM-safe defaultTargetPlatform through a pure @VisibleForTesting resolvePlatform() mapper over an exhaustive TargetPlatform switch. Preserves the 7 platform strings and isMobile semantics. Tests exercise every branch via debugDefaultTargetPlatformOverride.
Move FileImage(File(path)) behind a 2-file conditional import (stub returns null on web/WASM; io arm returns FileImage on native). Removes the dart:io import and the redundant kIsWeb branch from background_parser.
Run pana and fail the job if the is:wasm-ready tag is absent, guarding against dart:io re-entering the import graph. run-only step (no new pinned action).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR makes the Wind library WASM-ready by removing ChangesWASM-ready refactoring
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes the package WASM-compatible by removing unconditional dart:io usage from the library import graph (while preserving existing platform-string semantics), adding targeted tests for the new behavior, and enforcing the is:wasm-ready pana tag in CI.
Changes:
- Replaced
dart:io-based platform detection with a WASM-safedefaultTargetPlatformmapper (resolvePlatform()), plus exhaustive tests viadebugDefaultTargetPlatformOverride. - Moved absolute-path background image loading behind a conditional import (
file_image_stub.dartvsfile_image_io.dart) to avoiddart:ioon web/WASM. - Added a CI guard step to assert the
is:wasm-readypana tag and documented the fix in the changelog.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/parser/parsers/background_parser_test.dart | Adds coverage for the native (io) fileImageProvider() behavior. |
| test/core/platform_service_test.dart | Adds deterministic branch coverage for resolvePlatform() using debugDefaultTargetPlatformOverride. |
| lib/src/parser/parsers/file_image_stub.dart | Introduces web/WASM stub returning null to avoid File usage. |
| lib/src/parser/parsers/file_image_io.dart | Introduces native dart:io implementation returning FileImage. |
| lib/src/parser/parsers/background_parser.dart | Switches absolute-path handling to use conditional-imported fileImageProvider(). |
| lib/src/core/platform_service.dart | Removes dart:io dependency; resolves platform via defaultTargetPlatform (plus kIsWeb). |
| CHANGELOG.md | Documents the WASM-readiness change under “Fixed”. |
| .github/workflows/deploy.yml | Adds a pana-based CI assertion that is:wasm-ready remains present. |
Comment on lines
+72
to
+75
| dart pub global activate pana | ||
| dart pub global run pana --json --no-warning . > pana.json | ||
| python3 -c "import json,sys; tags=json.load(open('pana.json')).get('tags',[]); sys.exit(0 if 'is:wasm-ready' in tags else 'is:wasm-ready missing from pana tags')" | ||
|
|
Collaborator
Author
There was a problem hiding this comment.
Fixed in 8e511e0: the step now prints the present tags and exits with an explicit integer (sys.exit(0 if ok else 1)), so a failure shows which tags were found.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
CHANGELOG.md (1)
59-59: ⚡ Quick winAdd the issue reference to this changelog bullet.
Line 59 describes the behavior change but omits the relevant tracker reference; append
(#95)to match the repo’s changelog format contract.As per coding guidelines,
CHANGELOG.mdentries should include GitHub issue numbers ((#XX)) when relevant.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` at line 59, Update the CHANGELOG entry that starts with "WASM compatibility: removed `dart:io` from the library import graph (`platform_service.dart` now uses `defaultTargetPlatform`; absolute-path `bg-[/...]` image resolution moved behind a conditional import)." to append the issue reference "(`#95`)" at the end of that bullet so it reads with the tracker number; locate the bullet by that exact text and add the `(`#95`)` suffix to match the repo changelog format.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@CHANGELOG.md`:
- Line 59: Update the CHANGELOG entry that starts with "WASM compatibility:
removed `dart:io` from the library import graph (`platform_service.dart` now
uses `defaultTargetPlatform`; absolute-path `bg-[/...]` image resolution moved
behind a conditional import)." to append the issue reference "(`#95`)" at the end
of that bullet so it reads with the tracker number; locate the bullet by that
exact text and add the `(`#95`)` suffix to match the repo changelog format.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b64de13-5fe7-4e53-8cdb-2fc565f64d28
📒 Files selected for processing (8)
.github/workflows/deploy.ymlCHANGELOG.mdlib/src/core/platform_service.dartlib/src/parser/parsers/background_parser.dartlib/src/parser/parsers/file_image_io.dartlib/src/parser/parsers/file_image_stub.darttest/core/platform_service_test.darttest/parser/parsers/background_parser_test.dart
Address PR review: print the present tags and exit with an explicit integer instead of sys.exit(<string>), so a CI failure shows which tags were found. Also add the (#95) reference to the CHANGELOG entry.
Collaborator
Author
|
Addressed review feedback in 8e511e0:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WASM compatibility — pana 150 → 160
Removes
dart:iofrom thefluttersdk_windpublic import graph so the package becomesis:wasm-ready, raising pana Platform support 10/20 → 20/20 (total 150 → 160), with zero public-API change.Changes
platform_service.dart: platform detection now uses Flutter's WASM-safedefaultTargetPlatformvia a pure@visibleForTesting resolvePlatform()mapper (exhaustiveTargetPlatformswitch, nodefault). The 7 platform strings (ios/android/linux/macos/windows/unknown/web) andisMobilesemantics are preserved exactly (they back parser platform-prefixes + the cache key). Tests now cover every branch viadebugDefaultTargetPlatformOverride, dropping 4 host-dependentcoverage:ignore-linepragmas.background_parser.dart: the absolute-pathbg-[/...]FileImage(File(path))path moves behind a 2-file conditional import (file_image_stub.dartreturnsnullon web/WASM;file_image_io.dartreturnsFileImageon native).dart:ioand the redundantkIsWebbranch are removed; graceful web degradation unchanged.deploy.yml): arun:-only step asserts theis:wasm-readypana tag in the Lint & Test job, sodart:iocan't silently regress the score. No new pinned action (stays clear of the zizmor SHA-pin policy).### Fixed.Verification
dart analyze lib/ test/→ 0 issuesdart format→ no diffflutter test→ 1253 pass,./tool/coverage.sh 90→ 90.4%flutter pub publish --dry-run→ 0 warnings (post-commit)pana→ 160/160,is:wasm-ready: true, Platform support 20/20grep -rl "dart:io" lib/→ onlyfile_image_io.dart(the intended io arm)Planned + executed via
/ac:plan→/ac:execute(standard plan, reviewer + code-review both APPROVED).Summary by CodeRabbit
New Features
Bug Fixes
Chores