Skip to content

[dwds] Split hot reload callback into 2, disable breakpoints in changed files, and publish 24.3.10 #2606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 7, 2025

Conversation

srujzs
Copy link
Contributor

@srujzs srujzs commented Apr 4, 2025

dart-lang/sdk#60186

Breakpoints are currently broken when doing a hot reload when files change. They either are not re-mapped or point to the wrong location. For now, we should disable them in the changed libraries. In order to do this, we split the hot reload implementation into 2 in order to get the list of changed libraries. The call to hot reload will later use the cached list to call out to the embedder.

Note that this likely won't change any behavior. New JS files already don't have breakpoints, so this PR should be mostly about cleaning up bookkeeping.

…ed files, and publish 24.3.10

dart-lang/sdk#60186

Breakpoints are currently broken when doing a hot reload. They either
are not re-mapped or point to the wrong location. For now, we should
disable them in the changed libraries. In order to do this, we split
the hot reload implementation into 2 in order to get the list of changed
libraries. The call to hot reload will later use the cached list to
call out to the embedder.
@srujzs srujzs requested review from bkonyi and nshahan April 4, 2025 19:24
Copy link
Contributor

@nshahan nshahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems OK to me, but bundling the refactor into two callbacks and the disabling breakpoints into the same change might cause more churn something goes wrong and we want to revert? WDYT?

@srujzs
Copy link
Contributor Author

srujzs commented Apr 4, 2025

Seems OK to me, but bundling the refactor into two callbacks and the disabling breakpoints into the same change might cause more churn something goes wrong and we want to revert? WDYT?

It's possible, but the two callbacks are completely internal to DWDS so it'll be a cleanish revert. There's no reason to split the reload callback outside of disabling breakpoints currently, so it feels like a weird change to do separately.

I tested this against the hot reload tests we have in Flutter tools, so hopefully we should be able to land it without issues.

@srujzs
Copy link
Contributor Author

srujzs commented Apr 4, 2025

webdev tests are already failing for the same reasons, so this should be good to land. I'll wait until @bkonyi's review though and land this in Flutter on Monday.

@srujzs srujzs merged commit c8b1cfa into dart-lang:main Apr 7, 2025
43 of 47 checks passed
@srujzs
Copy link
Contributor Author

srujzs commented Apr 7, 2025

Going to land so we can get this in Flutter before the cutoff.

@bkonyi
Copy link
Collaborator

bkonyi commented Apr 7, 2025

LGTM.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Apr 9, 2025
Revisions updated by `dart tools/rev_sdk_deps.dart`.

core (https://github.com/dart-lang/core/compare/7a80178..af37fe5):
  af37fe54  2025-04-04  Lasse R.H. Nielsen  Adds `[Heap]PriorityQueue.of` constructor. (dart-lang/core#734)
  635dfa32  2025-04-03  Kevin Moore  [collection] explicitly make BoolList abstract interface (dart-lang/core#875)

ecosystem (https://github.com/dart-lang/ecosystem/compare/391a80c..7f6f1c1):
  7f6f1c1  2025-04-09  Daco Harkes  [firehose] Fix dart_apitool invocations with pub workspaces (dart-lang/ecosystem#355)
  0eb0349  2025-04-07  Moritz  Fix tagging in publishing workflow (again) (dart-lang/ecosystem#353)
  1ee8568  2025-04-07  Moritz  Update README.md (dart-lang/ecosystem#352)

http (https://github.com/dart-lang/http/compare/6fabf06..e4ddd3e):
  e4ddd3e  2025-04-07  Moritz  Merge pull request `#1750` from dart-lang/fixTags
  42b42e3  2025-04-07  Moritz  Fix tags
  54bf0f7  2025-04-07  Moritz  Merge pull request `#1748` from dart-lang/fixPublish
  84adca0  2025-04-04  Moritz  Merge pull request `#1432` from dart-lang/fixHealth
  8534a69  2025-04-04  Moritz  Remove ignore breaking
  b80436a  2025-04-04  Moritz  allow underscore
  949cd87  2025-04-04  Moritz  Fix publishing workflow
  996c5d1  2024-12-17  Moritz  ignore only for breaking changes
  e5321f7  2024-12-17  Moritz  Update .github/workflows/health.yaml
  f902d8a  2024-12-17  Moritz  typo
  35f6e9a  2024-12-17  Moritz  exclude websocket
  ca8caee  2024-12-17  Moritz  Ignore http
  621401e  2024-12-17  Moritz  remove ignore license
  ce20b2a  2024-12-17  Moritz  Fix health workflow

test (https://github.com/dart-lang/test/compare/c1fa1e6..8643fbf):
  8643fbf3  2025-04-09  Ömer Sinan Ağacan  Migrate from deprecated `dart:js`, `dart:js_util`, `package:js_util` to `dart:js_interop` (dart-lang/test#2478)

tools (https://github.com/dart-lang/tools/compare/b963bbf..d74f9e1):
  d74f9e13  2025-04-08  Loïc Sharma  [UA] Add a Flutter event for plugins injected into an iOS/macOS project. (dart-lang/tools#2062)
  f34228f8  2025-04-08  Kevin Moore  [graphs] fix readme CI badge (dart-lang/tools#2068)
  0102cd63  2025-04-08  Kevin Moore  [markdown] fix crash test (dart-lang/tools#2067)

webdev (https://github.com/dart-lang/webdev/compare/697f2f7..c8b1cfa):
  c8b1cfa9  2025-04-07  Srujan Gaddam  [dwds] Split hot reload callback into 2, disable breakpoints in changed files, and publish 24.3.10 (dart-lang/webdev#2606)
  8d8413f5  2025-04-04  Wdestroier  Support custom hostname and TLS options (dart-lang/webdev#2588)

Change-Id: Idbad02c2087ceb3c7d0f7efcf0721f4806475e8e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421542
Reviewed-by: Konstantin Shcheglov <[email protected]>
Auto-Submit: Devon Carew <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants