From 482444ffcadad8e4f11c3a2b6a4dfaee46435c91 Mon Sep 17 00:00:00 2001 From: Nicholas Shahan Date: Tue, 28 Apr 2026 16:04:43 -0700 Subject: [PATCH] [dwds] Use new hot restart API from embedder Start using the new `hotRestartBegin` and `hotRestartEnd` APIs from the embedder. These require the library bundle module system and allow for more customization in the integration of the hot restart operation. Specifically this allows for `$dartReloadModifiedModules` to decide which of the files provided from reloadedSources.json should actually be requested at this time and return the list of the actual requests so the restart logic in dwds knows what scripts it should await parse events for. Issue: https://github.com/dart-lang/webdev/issues/2826 --- dwds/lib/dwds.dart | 6 +- dwds/lib/src/debugging/chrome_inspector.dart | 3 +- dwds/lib/src/dwds_vm_client.dart | 63 ++++++++++++++----- dwds/lib/src/handlers/injector.dart | 4 +- dwds/lib/src/loaders/ddc_library_bundle.dart | 27 +------- dwds/lib/src/loaders/strategy.dart | 29 +++++++++ dwds/web/client.dart | 29 +++++++-- .../ddc_library_bundle_restarter.dart | 27 +++++++- dwds/web/reloader/manager.dart | 16 +++++ dwds/web/reloader/restarter.dart | 15 +++++ 10 files changed, 167 insertions(+), 52 deletions(-) diff --git a/dwds/lib/dwds.dart b/dwds/lib/dwds.dart index 2e178b52c..97c735fd8 100644 --- a/dwds/lib/dwds.dart +++ b/dwds/lib/dwds.dart @@ -30,7 +30,11 @@ export 'src/loaders/frontend_server_strategy_provider.dart' FrontendServerRequireStrategyProvider; export 'src/loaders/require.dart' show RequireStrategy; export 'src/loaders/strategy.dart' - show BuildSettings, LoadStrategy, ReloadConfiguration; + show + BuildSettings, + LoadStrategy, + ReloadConfiguration, + ReloadableLoadStrategy; export 'src/readers/asset_reader.dart' show AssetReader, PackageUriMapper; export 'src/readers/frontend_server_asset_reader.dart' show FrontendServerAssetReader; diff --git a/dwds/lib/src/debugging/chrome_inspector.dart b/dwds/lib/src/debugging/chrome_inspector.dart index 40a6472ff..47a920118 100644 --- a/dwds/lib/src/debugging/chrome_inspector.dart +++ b/dwds/lib/src/debugging/chrome_inspector.dart @@ -15,7 +15,6 @@ import 'package:dwds/src/debugging/libraries.dart'; import 'package:dwds/src/debugging/location.dart'; import 'package:dwds/src/debugging/metadata/provider.dart'; import 'package:dwds/src/debugging/remote_debugger.dart'; -import 'package:dwds/src/loaders/ddc_library_bundle.dart'; import 'package:dwds/src/readers/asset_reader.dart'; import 'package:dwds/src/utilities/conversions.dart'; import 'package:dwds/src/utilities/dart_uri.dart'; @@ -265,7 +264,7 @@ class ChromeAppInspector extends AppInspector { if (libraryUri == null) { throwInvalidParam('invoke', 'library uri is null'); } - return globalToolConfiguration.loadStrategy is DdcLibraryBundleStrategy + return globalToolConfiguration.loadStrategy.id == 'ddc-library-bundle' ? _evaluateLibraryMethodWithDdcLibraryBundle( libraryUri, selector, diff --git a/dwds/lib/src/dwds_vm_client.dart b/dwds/lib/src/dwds_vm_client.dart index aacfa597e..c5720ce1f 100644 --- a/dwds/lib/src/dwds_vm_client.dart +++ b/dwds/lib/src/dwds_vm_client.dart @@ -7,7 +7,6 @@ import 'dart:convert'; import 'package:dwds/src/config/tool_configuration.dart'; import 'package:dwds/src/events.dart'; -import 'package:dwds/src/loaders/ddc_library_bundle.dart'; import 'package:dwds/src/services/chrome/chrome_debug_exception.dart'; import 'package:dwds/src/services/chrome/chrome_debug_service.dart'; import 'package:dwds/src/services/chrome/chrome_proxy_service.dart'; @@ -441,11 +440,17 @@ final class ChromeDwdsVmClient (event) => event.kind == EventKind.kIsolateStart, ); try { + final isDdcLibraryBundle = + globalToolConfiguration.loadStrategy.id == 'ddc-library-bundle'; // If we should pause isolates on start, then only run main once we get a // resume event. final pauseIsolatesOnStart = chromeProxyService.pauseIsolatesOnStart; if (pauseIsolatesOnStart) { - _waitForResumeEventToRunMain(chromeProxyService); + if (isDdcLibraryBundle) { + _waitForResumeEventToEndHotRestart(chromeProxyService); + } else { + _waitForResumeEventToRunMain(chromeProxyService); + } } // Generate run id to hot restart all apps loaded into the tab. final runId = const Uuid().v4(); @@ -458,18 +463,18 @@ final class ChromeDwdsVmClient // TODO(srujzs): We don't do this for the AMD module format, should we? It // would require adding an extra parameter in the AMD strategy. As we're // planning to deprecate it, for now, do nothing. - final isDdcLibraryBundle = - globalToolConfiguration.loadStrategy is DdcLibraryBundleStrategy; - final computedReloadedSrcs = Completer(); - final reloadedSrcs = {}; - late StreamSubscription parsedScriptsSubscription; + if (isDdcLibraryBundle) { + final computedReloadedSrcs = Completer(); + final reloadedSrcs = {}; // Injected client should send a request to recreate the isolate after // the hot restart. The creation of the isolate should in turn wait // until all scripts are parsed. chromeProxyService.allowedToCreateIsolate = Completer(); final debugger = await chromeProxyService.debuggerFuture; - parsedScriptsSubscription = debugger.parsedScriptsController.stream + final parsedScriptsSubscription = debugger + .parsedScriptsController + .stream .listen((url) { computedReloadedSrcs.future.then((_) async { reloadedSrcs.remove(Uri.parse(url).normalizePath().path); @@ -479,14 +484,13 @@ final class ChromeDwdsVmClient } }); }); - } - logger.info('Issuing \$dartHotRestartDwds request'); - final remoteObject = await chromeProxyService.inspector.jsEvaluate( - '\$dartHotRestartDwds(\'$runId\', $pauseIsolatesOnStart);', - awaitPromise: true, - returnByValue: true, - ); - if (isDdcLibraryBundle) { + logger.info('Issuing \$dartHotRestartBeginDwds request'); + final remoteObject = await chromeProxyService.inspector.jsEvaluate( + '\$dartHotRestartBeginDwds(\'$runId\', $pauseIsolatesOnStart);', + awaitPromise: true, + returnByValue: true, + ); + logger.info('\$dartHotRestartBeginDwds request complete.'); final reloadedSrcModuleLibraries = (remoteObject.value as List) .cast(); for (final srcModuleLibrary in reloadedSrcModuleLibraries) { @@ -504,9 +508,15 @@ final class ChromeDwdsVmClient await chromeProxyService.allowedToCreateIsolate.future; await parsedScriptsSubscription.cancel(); } else { + logger.info('Issuing \$dartHotRestartDwds request.'); + final remoteObject = await chromeProxyService.inspector.jsEvaluate( + '\$dartHotRestartDwds(\'$runId\', $pauseIsolatesOnStart);', + awaitPromise: true, + returnByValue: true, + ); assert(remoteObject.value == null); + logger.info('\$dartHotRestartDwds request complete.'); } - logger.info('\$dartHotRestartDwds request complete.'); } on WipError catch (exception) { final code = exception.error?['code']; final message = exception.error?['message']; @@ -545,6 +555,25 @@ final class ChromeDwdsVmClient }); } + /// Waits for the isolate to start after a hot restart and then issues the + /// request to finish the hot restart operation. + void _waitForResumeEventToEndHotRestart( + ChromeProxyService chromeProxyService, + ) { + StreamSubscription? resumeEventsSubscription; + resumeEventsSubscription = chromeProxyService.resumeAfterRestartEventsStream + .listen((_) async { + await resumeEventsSubscription!.cancel(); + logger.info('Issuing \$dartHotRestartEndDwds request.'); + await chromeProxyService.inspector.jsEvaluate( + '\$dartHotRestartEndDwds();', + awaitPromise: true, + returnByValue: true, + ); + logger.info('\$dartHotRestartEndDwds request complete.'); + }); + } + Future> _fullReload( ChromeProxyService chromeProxyService, ) async { diff --git a/dwds/lib/src/handlers/injector.dart b/dwds/lib/src/handlers/injector.dart index f0f6e1b92..5523a43fe 100644 --- a/dwds/lib/src/handlers/injector.dart +++ b/dwds/lib/src/handlers/injector.dart @@ -9,7 +9,7 @@ import 'dart:io'; import 'package:crypto/crypto.dart'; import 'package:dwds/src/config/tool_configuration.dart'; import 'package:dwds/src/handlers/injected_client_js.dart'; -import 'package:dwds/src/loaders/ddc_library_bundle.dart'; +import 'package:dwds/src/loaders/strategy.dart'; import 'package:dwds/src/version.dart'; import 'package:logging/logging.dart'; import 'package:shelf/shelf.dart'; @@ -189,7 +189,7 @@ Future _injectedClientSnippet( final buildSettings = loadStrategy.buildSettings; final appMetadata = globalToolConfiguration.appMetadata; final debugSettings = globalToolConfiguration.debugSettings; - final reloadedSourcesPath = loadStrategy is DdcLibraryBundleStrategy + final reloadedSourcesPath = loadStrategy is ReloadableLoadStrategy ? 'window.\$reloadedSourcesPath = "${loadStrategy.reloadedSourcesUri}";\n' : ''; diff --git a/dwds/lib/src/loaders/ddc_library_bundle.dart b/dwds/lib/src/loaders/ddc_library_bundle.dart index 31b483e4e..2404fb956 100644 --- a/dwds/lib/src/loaders/ddc_library_bundle.dart +++ b/dwds/lib/src/loaders/ddc_library_bundle.dart @@ -14,7 +14,8 @@ import 'package:shelf/shelf.dart'; // TODO(srujzs): This is mostly a copy of `DdcStrategy`. Some of the // functionality in here may not make sense with the library bundle format yet. -class DdcLibraryBundleStrategy extends LoadStrategy { +class DdcLibraryBundleStrategy extends LoadStrategy + implements ReloadableLoadStrategy { @override final ReloadConfiguration reloadConfiguration; @@ -104,29 +105,7 @@ class DdcLibraryBundleStrategy extends LoadStrategy { final BuildSettings _buildSettings; - /// The [Uri] of the file that contains a JSONified list of maps which follows - /// the following format: - /// - /// ```json - /// [ - /// { - /// "src": "/", - /// "module": "", - /// "libraries": ["", ""], - /// }, - /// ] - /// ``` - /// - /// `src`: A string that corresponds to the file path containing a DDC library - /// bundle. - /// `module`: The name of the library bundle in `src`. - /// `libraries`: An array of strings containing the libraries that were - /// compiled in `src`. - /// - /// This is needed for hot reloads and restarts in order to tell the module - /// loader what files need to be loaded and what libraries need to be - /// reloaded. The contents of the file this [Uri] points to should be updated - /// whenever a hot reload or hot restart is executed. + @override final Uri? reloadedSourcesUri; /// When enabled, injects the script loader into the bootstrapper from diff --git a/dwds/lib/src/loaders/strategy.dart b/dwds/lib/src/loaders/strategy.dart index 9be33e21f..b3f849d56 100644 --- a/dwds/lib/src/loaders/strategy.dart +++ b/dwds/lib/src/loaders/strategy.dart @@ -13,6 +13,35 @@ import 'package:dwds/src/utilities/dart_uri.dart'; import 'package:path/path.dart' as p; import 'package:shelf/shelf.dart'; +/// A load strategy that supports reading from a URI to find the sources needed +/// to complete the hot reload or hot restart operation. +abstract class ReloadableLoadStrategy implements LoadStrategy { + /// The [Uri] of the file that contains a JSONified list of maps which follows + /// the following format: + /// + /// ```json + /// [ + /// { + /// "src": "/", + /// "module": "", + /// "libraries": ["", ""], + /// }, + /// ] + /// ``` + /// + /// `src`: A string that corresponds to the file path containing a DDC library + /// bundle. + /// `module`: The name of the library bundle in `src`. + /// `libraries`: An array of strings containing the libraries that were + /// compiled in `src`. + /// + /// This is needed for hot reloads and restarts in order to tell the module + /// loader what files need to be loaded and what libraries need to be + /// reloaded. The contents of the file this [Uri] points to should be updated + /// whenever a hot reload or hot restart is executed. + Uri? get reloadedSourcesUri; +} + abstract class LoadStrategy { final AssetReader _assetReader; final String? _packageConfigPath; diff --git a/dwds/web/client.dart b/dwds/web/client.dart index 059e054c5..882d8ec59 100644 --- a/dwds/web/client.dart +++ b/dwds/web/client.dart @@ -90,6 +90,12 @@ Future? main() { return manager.hotReloadEnd().toJS; }.toJS; + hotRestartBeginJs = () { + return manager.hotRestartBegin(hotReloadReloadedSourcesPath).toJS; + }.toJS; + + hotRestartEndJs = manager.hotRestartEnd.toJS; + Completer? readyToRunMainCompleter; hotRestartJs = (String runId, [bool? pauseIsolatesOnStart]) { @@ -523,10 +529,17 @@ Future handleWebSocketHotRestartRequest( final requestId = event.id; try { final runId = const Uuid().v4(); - await manager.hotRestart( - runId: runId, - reloadedSourcesPath: hotRestartReloadedSourcesPath, - ); + if (manager.supportsTwoPhaseHotRestart) { + await manager.hotRestartBegin(hotRestartReloadedSourcesPath!); + manager.hotRestartEnd(); + } else { + // TODO(nshahan): Remove after migrating to hotRestartBegin/hotRestartEnd. + // https://github.com/dart-lang/webdev/issues/2826 + await manager.hotRestart( + runId: runId, + reloadedSourcesPath: hotRestartReloadedSourcesPath, + ); + } _sendHotRestartResponse(clientSink, requestId, success: true); } catch (e) { _sendHotRestartResponse( @@ -597,6 +610,12 @@ external set hotReloadStartJs(JSFunction cb); @JS(r'$dartHotReloadEndDwds') external set hotReloadEndJs(JSFunction cb); +@JS(r'$dartHotRestartBeginDwds') +external set hotRestartBeginJs(JSFunction cb); + +@JS(r'$dartHotRestartEndDwds') +external set hotRestartEndJs(JSFunction cb); + @JS(r'$reloadedSourcesPath') external String? get _reloadedSourcesPath; @@ -612,6 +631,8 @@ String get hotReloadReloadedSourcesPath { } /// Debugger-initiated hot restart. +// TODO(nshahan): Remove after migrating to hotRestartBegin/hotRestartEnd. +// https://github.com/dart-lang/webdev/issues/2826 @JS(r'$dartHotRestartDwds') external set hotRestartJs(JSFunction cb); diff --git a/dwds/web/reloader/ddc_library_bundle_restarter.dart b/dwds/web/reloader/ddc_library_bundle_restarter.dart index c0335b934..19d0a51ff 100644 --- a/dwds/web/reloader/ddc_library_bundle_restarter.dart +++ b/dwds/web/reloader/ddc_library_bundle_restarter.dart @@ -15,7 +15,14 @@ external _DartDevEmbedder get _dartDevEmbedder; extension type _DartDevEmbedder._(JSObject _) implements JSObject { external _Debugger get debugger; - external JSPromise hotRestart(); + external JSPromise?> hotRestart([ + JSArray? reloadedSources, + ]); + external JSPromise> hotRestartBegin([ + JSArray? reloadedSources, + ]); + external JSAny? hotRestartEnd(); + external JSPromise hotReload( JSArray filesToLoad, JSArray librariesToReload, @@ -64,7 +71,7 @@ extension on JSArray { external void push(JSString value); } -class DdcLibraryBundleRestarter implements Restarter { +class DdcLibraryBundleRestarter implements Restarter, TwoPhaseRestarter { JSFunction? _capturedHotReloadEndCallback; Future _runMainWhenReady( @@ -125,6 +132,22 @@ class DdcLibraryBundleRestarter implements Restarter { return (true, srcModuleLibraries.jsify() as JSArray); } + @override + Future> hotRestartBegin(String reloadedSourcesPath) async { + await _dartDevEmbedder.debugger.maybeInvokeFlutterDisassemble(); + final srcModuleLibraries = await _getSrcModuleLibraries( + reloadedSourcesPath, + ); + final jsFilesToRequest = srcModuleLibraries.jsify() as JSArray; + final JSArray requestedJsFiles = await _dartDevEmbedder + .hotRestartBegin(jsFilesToRequest) + .toDart; + return requestedJsFiles; + } + + @override + void hotRestartEnd() => _dartDevEmbedder.hotRestartEnd(); + @override Future> hotReloadStart(String reloadedSourcesPath) async { final filesToLoad = JSArray(); diff --git a/dwds/web/reloader/manager.dart b/dwds/web/reloader/manager.dart index 8c3f52a85..f1f7ac154 100644 --- a/dwds/web/reloader/manager.dart +++ b/dwds/web/reloader/manager.dart @@ -64,6 +64,22 @@ class ReloadingManager { return result.$2; } + bool get supportsTwoPhaseHotRestart => _restarter is TwoPhaseRestarter; + + Future> hotRestartBegin(String reloadedSourcesPath) async { + final requestedSources = await (_restarter as TwoPhaseRestarter) + .hotRestartBegin(reloadedSourcesPath); + // Notify package:dwds that the isolate is exiting and a new isolate will + // be created. + _beforeRestart(); + _afterRestart(true); + return requestedSources; + } + + void hotRestartEnd() { + (_restarter as TwoPhaseRestarter).hotRestartEnd(); + } + /// After a previous call to [hotReloadStart], completes the hot /// reload by pushing the libraries into the Dart runtime. Future hotReloadEnd() async { diff --git a/dwds/web/reloader/restarter.dart b/dwds/web/reloader/restarter.dart index 689491146..27604ab45 100644 --- a/dwds/web/reloader/restarter.dart +++ b/dwds/web/reloader/restarter.dart @@ -4,6 +4,19 @@ import 'dart:js_interop'; +/// A Restarter that supports a hot restart over two phases. +abstract class TwoPhaseRestarter implements Restarter { + /// Starts a hot restart operation. + /// + /// Passes the [reloadedSourcesPath] through to the `DartDevEmbedder` and + /// bubbles up the returned array of scripts that were actually requested. + Future> hotRestartBegin(String reloadedSourcesPath); + + /// Finishes the hot restart operation that must have been previously started by + /// [hotRestartBegin]. + void hotRestartEnd(); +} + abstract class Restarter { /// Attempts to perform a hot restart. /// @@ -30,6 +43,8 @@ abstract class Restarter { /// Returns a record containing whether the hot restart succeeded and either /// the JS version of the list of maps from [reloadedSourcesPath] if /// [reloadedSourcesPath] is non-null and null otherwise. + // TODO(nshahan): Remove after migrating to hotRestartBegin/hotRestartEnd. + // https://github.com/dart-lang/webdev/issues/2826 Future<(bool, JSArray?)> restart({ String? runId, Future? readyToRunMain,