[CP-beta]Use relative path for reloadedSourcesUri and reloaded modules#185540
Conversation
…r#184598) Fixes flutter#184060 The app may be opened in a separate domain than the server. Therefore, we need to provide relative paths/URIs to the root instead of including the server URI. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
|
@srujzs please fill out the PR description above, afterwards the release team will review this request. |
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
There was a problem hiding this comment.
Code Review
This pull request updates the WebAssetServer to use relative paths for reloaded sources in the DDC library bundle, ensuring the application can locate them even when served from different domains. The changes replace the static filename with a reloadedSourcesUri and update the JSON output to include a leading slash for module paths. A new test case has been added to verify these relative paths. Feedback includes a suggestion to mark the new static Uri as final and a correction for Dart formatting in a json.decode call.
| // Use relative path for the URI so the app can still find it even if it's in | ||
| // a different domain than the server. | ||
| @visibleForTesting | ||
| static Uri reloadedSourcesUri = Uri.parse('reloaded_sources.json'); |
There was a problem hiding this comment.
The reloadedSourcesUri field is a static mutable variable. Since it represents a constant relative path used across the server, it should be marked as final to prevent accidental reassignment and ensure consistency.
| static Uri reloadedSourcesUri = Uri.parse('reloaded_sources.json'); | |
| static final Uri reloadedSourcesUri = Uri.parse('reloaded_sources.json'); |
| json.decode( | ||
| utf8.decode(_webMemoryFS.metadataFiles['$relativeModulePath.metadata']!.toList()), | ||
| ) | ||
| as Map<String, dynamic>, |
There was a problem hiding this comment.
The indentation and formatting of the json.decode call do not appear to follow standard Dart style (as enforced by dart format). The nested utf8.decode call has excessive indentation.
| json.decode( | |
| utf8.decode(_webMemoryFS.metadataFiles['$relativeModulePath.metadata']!.toList()), | |
| ) | |
| as Map<String, dynamic>, | |
| json.decode( | |
| utf8.decode(_webMemoryFS.metadataFiles['$relativeModulePath.metadata']!.toList()), | |
| ) as Map<String, dynamic>, |
References
- All Dart code is formatted using
dart format. (link)
justinmc
left a comment
There was a problem hiding this comment.
CPLGTM 👍 . Apologies that I missed this PR earlier in the week, it will likely not be released until next week.
|
One weird test failure, possibly an infra issue. I've rerun it. |
No worries, we didn't realize we wanted to cherry-pick this until very recently anyways. :) |
e46d7dd
into
flutter:flutter-3.44-candidate.0
This pull request is created by automatic cherry pick workflow
Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request.
Issue Link:
#184060
Impact Description:
Hot reload on web fails when opening the app in another domain than the server. This impacts web development.
Changelog Description:
[flutter/184060] When users open an app in a different domain, hot reload will fail to succeed on Web.
Workaround:
None. Users cannot control the format of the sources sent to DWDS.
Risk:
Test Coverage:
Validation Steps:
Beyond the added test, you can manually test by running
flutter run -d web-server --web-hostname=0.0.0.0 --web-port=8080, opening<ip_address>:8080on another computer on the same network, and checking that hot reload changes are visible on the other computer.