Wait until all scripts are parsed before recomputing metadata on a hot reload

dwds/CHANGELOG.md

@@ -1,5 +1,8 @@
 ## 24.4.1-wip
 
+- Fixed an issue where we didn't wait until all scripts were parsed before
+  recomputing metadata on a hot reload.
+
 ## 24.4.0
 
 - Added support for breakpoint registering on a hot reload with the DDC library bundle format using PausePostRequests.

dwds/lib/src/debugging/debugger.dart

@@ -56,6 +56,9 @@ class Debugger extends Domain {
 
   int _frameErrorCount = 0;
 
+  final StreamController<String> parsedScriptsController =
+      StreamController.broadcast();
+
   Debugger._(
     this._remoteDebugger,
     this._streamNotify,
@@ -489,6 +492,9 @@ class Debugger extends Domain {
     final scriptPath = _pathForChromeScript(e.script.url);
     if (scriptPath != null) {
       chromePathToRuntimeScriptId[scriptPath] = e.script.scriptId;
+      if (parsedScriptsController.hasListener) {
+        parsedScriptsController.sink.add(e.script.url);
+      }
     }
   }
 

dwds/lib/src/debugging/modules.dart

@@ -98,9 +98,6 @@ class Modules {
   ) async {
     final serverPath = await globalToolConfiguration.loadStrategy
         .serverPathForModule(entrypoint, module);
-    // TODO(srujzs): We should wait until all scripts are parsed before
-    // accessing after a hot reload. See
-    // https://github.com/dart-lang/webdev/issues/2640.
     return chromePathToRuntimeScriptId[serverPath];
   }
 

dwds/lib/src/loaders/ddc_library_bundle.dart

@@ -110,14 +110,16 @@ class DdcLibraryBundleStrategy extends LoadStrategy {
   /// ```json
   /// [
   ///   {
-  ///     "src": "<file_name>",
+  ///     "src": "<base_uri>/<file_name>",
+  ///     "module": "<module_name>",
   ///     "libraries": ["<lib1>", "<lib2>"],
   ///   },
   /// ]
   /// ```
   ///
   /// `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`.
   ///

dwds/lib/src/services/chrome_proxy_service.dart

@@ -1221,15 +1221,42 @@ class ChromeProxyService implements VmServiceInterface {
   Future<void> _performClientSideHotReload(String isolateId) async {
     _logger.info('Attempting a hot reload');
 
+    final debugger = await debuggerFuture;
+    final reloadedSrcs = <String>{};
+    final computedReloadedSrcs = Completer<void>();
+    final parsedAllReloadedSrcs = Completer<void>();
+    // Wait until all the reloaded scripts are parsed before we reinitialize
+    // metadata below.
+    final parsedScriptsSubscription = debugger.parsedScriptsController.stream
+        .listen((url) {
+          computedReloadedSrcs.future.then((_) {
+            reloadedSrcs.remove(Uri.parse(url).normalizePath().path);
+            if (reloadedSrcs.isEmpty) {
+              parsedAllReloadedSrcs.complete();
+            }
+          });
+        });
+
     // Initiate a hot reload.
     _logger.info('Issuing \$dartHotReloadStartDwds request');
     final remoteObject = await inspector.jsEvaluate(
       '\$dartHotReloadStartDwds();',
       awaitPromise: true,
       returnByValue: true,
     );
-    final reloadedModulesToLibraries =
-        (remoteObject.value as Map).cast<String, List>();
+    final reloadedSrcModuleLibraries = (remoteObject.value as List).cast<Map>();
+    final reloadedModulesToLibraries = <String, List<String>>{};
+    for (final srcModuleLibrary in reloadedSrcModuleLibraries) {
+      final srcModuleLibraryCast = srcModuleLibrary.cast<String, Object>();
+      reloadedSrcs.add(
+        Uri.parse(srcModuleLibraryCast['src'] as String).normalizePath().path,
+      );
+      reloadedModulesToLibraries[srcModuleLibraryCast['module'] as String] =
+          (srcModuleLibraryCast['libraries'] as List).cast<String>();
+    }
+    computedReloadedSrcs.complete();
+    if (reloadedSrcs.isNotEmpty) await parsedAllReloadedSrcs.future;
+    await parsedScriptsSubscription.cancel();
 
     if (!pauseIsolatesOnStart) {
       // Finish hot reload immediately.

dwds/test/common/chrome_proxy_service_common.dart

@@ -2225,27 +2225,6 @@ void runTests({
       );
     });
 
-    test('reloadSources', () async {
-      final service = context.service;
-      final vm = await service.getVM();
-      final isolateId = vm.isolates!.first.id!;
-      final report = await service.reloadSources(isolateId);
-      // TODO(srujzs): This naturally fails regardless of the module format
-      // because we didn't set up prerequisite state (the reload scripts). We
-      // should create new tests for hot reload within DWDS and remove this
-      // test.
-      expect(report.success, false);
-      // Make sure that a notice was provided in the expected format.
-      final notices = report.json?['notices'];
-      expect(notices, isNotNull);
-      expect(notices is List, isTrue);
-      final noticeList = (notices as List).cast<Map>();
-      expect(noticeList, isNotEmpty);
-      final message = noticeList[0]['message'];
-      expect(message, isNotNull);
-      expect(message is String && message.isNotEmpty, isTrue);
-    });
-
     test('setIsolatePauseMode', () async {
       final service = context.service;
       final vm = await service.getVM();

dwds/test/fixtures/context.dart

@@ -339,11 +339,9 @@ class TestContext {
             _hostname = appMetadata.hostname;
             await webRunner.run(
               frontendServerFileSystem,
-              _hostname,
-              assetServerPort,
-              filePathToServe,
-              initialCompile: true,
-              fullRestart: false,
+              hostname: _hostname,
+              port: assetServerPort,
+              index: filePathToServe,
             );
 
             if (testSettings.enableExpressionEvaluation) {
@@ -433,6 +431,10 @@ class TestContext {
       final appConnectionCompleter = Completer();
       final connection = ChromeConnection('localhost', debugPort);
 
+      // TODO(srujzs): In the case of the frontend server, it doesn't make sense
+      // that we initialize a new HTTP server instead of reusing the one in
+      // `TestAssetServer`. We should instead use that one to align with Flutter
+      // tools.
       _testServer = await TestServer.start(
         debugSettings: debugSettings.copyWith(
           expressionCompiler: expressionCompiler,
@@ -590,13 +592,9 @@ class TestContext {
   }
 
   Future<void> recompile({required bool fullRestart}) async {
-    await webRunner.run(
-      frontendServerFileSystem,
-      _hostname,
-      await findUnusedPort(),
-      webCompatiblePath([project.directoryToServe, project.filePathToServe]),
-      initialCompile: false,
+    await webRunner.rerun(
       fullRestart: fullRestart,
+      fileServerUri: Uri.parse('http://${testServer.host}:${testServer.port}'),
     );
     return;
   }