Wait until all scripts are parsed before recomputing metadata on a hot reload #2650
Conversation
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
dart-lang@2172ba7 added support to run tests in a temporary directory. This results in two flaky issues: 1. On Windows, build_daemon tests may fail to delete the temp directory because the process may have not been torn down yet, so it may still be accessing the file system. There was an initial retry after 1 second, but that appears to be not enough looking at a recent test run. 2. If a test times out, its tearDown may not be called. In this case, the ResidentWebRunner in frontend_server may not restore the current directory in the LocalFileSystem. This leads to cascading failures in subsequent tests due to no longer being in a path that contains 'webdev'. See https://github.com/dart-lang/webdev/actions/runs/15989286213/job/45099373212?pr=2641 for an example. See dart-lang/test#897 as well for tracking work to call tearDown on timeouts. To address the above issues: 1. Increase the delay between the two tries and assert this only occurs on Windows. 2. Cache the current directory so that it can be used in utilities.dart with the same (correct) value every time.
This reverts commit 3bd1448.
…t reload Fixes dart-lang#2640 On a hot reload, we wait until all the scripts are downloaded, but we don't wait until all of them are parsed and a script ID is created for them to refer to. This then results in incorrect metadata being computed. Instead, return the srcs that are being loaded during a hot reload, use a controller to determine when scripts are parsed, and only compute metadata once we have parsed all the reloaded scripts. In order to compare the parsed scripts' URLs with the reloaded scripts' URLs, we now require full URLs in the hot reload sources metadata. This is already true in Flutter tools, so this just canonicalizes that and modifies the tests to do the same. To be consistent, hot restart also provides the full URL in the DDC library bundle format and the bootstrap is modified to reflect that (and to clean up some unused code around module tracking).
srujzs
added a commit
to srujzs/flutter
that referenced
this pull request
Jul 17, 2025
We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Also cleans up some code that was never used around tracking modules in the bootstrap. Related PR: dart-lang/webdev#2650
9 tasks
|
Looks like there's some existing redness (see |
nshahan
approved these changes
Jul 18, 2025
… a TODO to remove a test
github-merge-queue bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jul 21, 2025
We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] 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.
github-merge-queue bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jul 21, 2025
We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] 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.
|
Failures are consistent with existing failures: https://github.com/dart-lang/webdev/actions/runs/16394079815 (both the specific tests and their failure logs) so I'm going to go ahead and land this. |
azatech
pushed a commit
to azatech/flutter
that referenced
this pull request
Jul 28, 2025
…2271) We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] 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.
ksokolovskyi
pushed a commit
to ksokolovskyi/flutter
that referenced
this pull request
Aug 19, 2025
…2271) We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] 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.
mboetger
pushed a commit
to mboetger/flutter
that referenced
this pull request
Sep 18, 2025
…2271) We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] 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.
korca0220
pushed a commit
to korca0220/flutter
that referenced
this pull request
Sep 22, 2025
…2271) We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] 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.
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.
Fixes #2640
On a hot reload, we wait until all the scripts are downloaded, but we
don't wait until all of them are parsed and a script ID is created for
them to refer to. This then results in incorrect metadata being computed.
Instead, return the srcs that are being loaded during a hot reload,
use a controller to determine when scripts are parsed, and only
compute metadata once we have parsed all the reloaded scripts.
In order to compare the parsed scripts' URLs with the reloaded scripts'
URLs, we now require full URLs in the hot reload sources metadata.
This is already true in Flutter tools, so this just canonicalizes that
and modifies the tests to do the same.
To be consistent, hot restart also provides the full URL in the DDC
library bundle format and the bootstrap is modified to reflect that.