Skip to content

Updated DWDS to include a boolean flag that enables debugging support when set to true #2601

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Mar 21, 2025
Merged

Updated DWDS to include a boolean flag that enables debugging support when set to true #2601

merged 8 commits into from
Mar 21, 2025

Conversation

jyameo
Copy link
Contributor

@jyameo jyameo commented Mar 19, 2025 •
edited
Loading

Updated DWDS to include a boolean flag that enables debugging support when set to true (default behavior). This flag will enable us to support running a Flutter app on a web-server with the new DDC library bundle format. However, it will not support debugging, as debugging features are not compatible with this configuration.

@jyameo jyameo requested review from srujzs and bkonyi March 19, 2025 18:41
@jyameo jyameo changed the title Updated DWDS to include a boolean flag that enables debugging support only when set to true Updated DWDS to include a boolean flag that enables debugging support when set to true Mar 19, 2025
bkonyi
bkonyi reviewed Mar 20, 2025
View reviewed changes
Copy link
Collaborator

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

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

Is it possible to write tests here to ensure hot reload works with this change, or will those need to be added in the Flutter repo?

dwds/lib/dart_web_debug_service.dart
@@ -66,6 +66,7 @@ class Dwds {
required Stream<BuildResult> buildResults,
required ConnectionProvider chromeConnection,
required ToolConfiguration toolConfiguration,
bool enableDebuggingSupport = true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

It'd be great if we had documentation explaining what this does and what the implications are. Unfortunately, it doesn't look like we have much documentation here. Can we at least add some to the DwdsInjector constructor for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the docstring for DwdsInjector to include more details.

@jyameo
Copy link
Contributor Author

jyameo commented Mar 20, 2025 •
edited
Loading

Is it possible to write tests here to ensure hot reload works with this change, or will those need to be added in the Flutter repo?

This change separates the loading of the injected client from the rest of the code that handles source loading. It does not affect how hot reload currently works when running flutter run -d chrome --web-experimental-hot-reload. However, when the flag is set to false, the app will be loaded without debugging (i.e., no hot reload), which is what we want for flutter run -d web-server --web-experimental-hot-reload for now.

Supporting hot restart/hot reload for web-server in the future would require a completely different mechanism, as hot reload currently relies on Chrome DevTools. Since adding hot reload support for web-server is a non-trivial change, we decided with the web/ddc team that the immediate goal is to get everything into a presentable state before the cutoff for the next Dart stable release. It was either this approach or landing a change to just throw an error if someone tries to do flutter run -d web-server --web-experimental-hot-reload. We opted for the former approach.

Once this PR is published, I'll land another change in Flutter tools to pass this flag to DWDS based on the environment specified by the user (i.e., chrome vs. web-server).

We already have some existing hot-reload tests that Srujan wrote in PR #162498.

@jyameo jyameo requested a review from bkonyi March 21, 2025 17:41
@jyameo jyameo merged commit 5c805f1 into dart-lang:main Mar 21, 2025
43 of 47 checks passed
This was referenced Mar 21, 2025
Closed
Merged
Merged
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Mar 24, 2025
@jyameo
Set injectDebuggingSupportCode in DWDS based on specified device ID (#…
e52eade 
…165820)

This change ensures that `injectDebuggingSupportCode` in DWDS is set
based on the specified device ID, defaulting to Chrome. This complements
the changes made in dart-lang/webdev#2601 and
ensures that debugging support is correctly configured for different
environments.

fixes dart-lang/sdk#60289
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Mar 24, 2025
@jyameo
Set injectDebuggingSupportCode in DWDS based on specified device ID (#…
45d86cf 
…165820)

This change ensures that `injectDebuggingSupportCode` in DWDS is set
based on the specified device ID, defaulting to Chrome. This complements
the changes made in dart-lang/webdev#2601 and
ensures that debugging support is correctly configured for different
environments.

fixes dart-lang/sdk#60289
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Apr 2, 2025
@devoncarew
[deps] rev core, dartdoc, ecosystem, http, i18n, shelf, test, webdev
d56749c 
Revisions updated by `dart tools/rev_sdk_deps.dart`.

core (https://github.com/dart-lang/core/compare/61e6771..a6e81e0):
  a6e81e0b  2025-03-31  Lasse R.H. Nielsen  Make URL strategy better at recognizing URLs. (dart-lang/core#873)
  379e9c2e  2025-03-25  dependabot[bot]  Bump dart-lang/setup-dart from 1.7.0 to 1.7.1 in the github-actions group (dart-lang/core#869)

dartdoc (https://github.com/dart-lang/dartdoc/compare/d400676..cbc1885):
  cbc18857  2025-03-25  Sam Rawlins  Improve the error message when an import cannot be resolved (dart-lang/dartdoc#4023)
  18a7631d  2025-03-25  Parker Lougheed  Place footer below main content (dart-lang/dartdoc#4014)
  f41bd3ab  2025-03-24  Sam Rawlins  Improve failure when import cannot be found (dart-lang/dartdoc#4022)

ecosystem (https://github.com/dart-lang/ecosystem/compare/2317263..391a80c):
  391a80c  2025-04-01  Brian Wilkerson  Update prompts.dart (dart-lang/ecosystem#349)
  6a89899  2025-04-01  Brian Wilkerson  Update create_tuning_data.dart (dart-lang/ecosystem#350)

http (https://github.com/dart-lang/http/compare/1b6e28d..e988925):
  e988925  2025-03-31  Brian Quinlan  Remove .gitignored files (dart-lang/http#1728)
  29c5733  2025-03-25  Brian Quinlan  Improve the reliability of cronet_http/ok_http workflows (dart-lang/http#1736)
  dd56b86  2025-03-21  Brian Quinlan  Remove cronet_http.impl (dart-lang/http#1734)
  7f5ad18  2025-03-21  Brian Quinlan  Adding missing license headers (dart-lang/http#1735)
  fedead7  2025-03-20  Brian Quinlan  [cronet_http] Prepare to release 1.3.3 (dart-lang/http#1733)

i18n (https://github.com/dart-lang/i18n/compare/d9cce0b..2701040):
  2701040  2025-04-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/i18n#959)
  5101533  2025-04-01  Moritz  Fix hashes.dart formatting (dart-lang/i18n#960)

shelf (https://github.com/dart-lang/shelf/compare/2af8529..082d3ac):
  082d3ac  2025-04-01  dependabot[bot]  Bump actions/cache from 4.2.2 to 4.2.3 in the github-actions group (dart-lang/shelf#475)

test (https://github.com/dart-lang/test/compare/9e349d0..c1fa1e6):
  c1fa1e69  2025-04-01  dependabot[bot]  Bump the github-actions group with 3 updates (dart-lang/test#2476)
  31a2d64c  2025-03-31  Nate Bosch  Sort reporter options (dart-lang/test#2475)

webdev (https://github.com/dart-lang/webdev/compare/302b6db..697f2f7):
  697f2f7f  2025-03-24  Jessy Yameogo  renamed DWDS injector parameter (dart-lang/webdev#2602)
  af33df71  2025-03-21  Tristan Ross  Add offline mode (dart-lang/webdev#2483)
  5c805f14  2025-03-21  Jessy Yameogo  Updated DWDS to include a boolean flag that enables debugging support when set to true (dart-lang/webdev#2601)

Change-Id: I7af88306b77efa3dab2bdef0d7543663317448f1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/419682
Auto-Submit: Devon Carew <[email protected]>
Reviewed-by: Konstantin Shcheglov <[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
Reviewers

@bkonyi bkonyi bkonyi approved these changes

@srujzs srujzs Awaiting requested review from srujzs

Assignees
No one assigned
Labels
package:dwds prepare-release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jyameo @bkonyi