Improve HMR (#10017)

parcel-bundler · Nov 24, 2024 · 79c8d98 · 79c8d98
1 parent a53f8f3
commit 79c8d98
Show file tree

Hide file tree

Showing 17 changed files with 160 additions and 31 deletions.
diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js
@@ -530,6 +530,14 @@ export default class AssetGraph extends ContentGraph<AssetGraphNode> {
           ...depNode.value.meta,
           ...existing.value.resolverMeta,
         };
+        depNode.value.resolverMeta = existing.value.resolverMeta;
+      }
+      if (
+        existing?.type === 'dependency' &&
+        existing.value.resolverPriority != null
+      ) {
+        depNode.value.priority = existing.value.resolverPriority;
+        depNode.value.resolverPriority = existing.value.resolverPriority;
       }
       let dependentAsset = dependentAssets.find(
         a => a.uniqueKey === dep.specifier,

diff --git a/packages/core/core/src/requests/PathRequest.js b/packages/core/core/src/requests/PathRequest.js
@@ -315,7 +315,8 @@ export class ResolverRunner {
           }
 
           if (result.priority != null) {
-            dependency.priority = Priority[result.priority];
+            dependency.priority = dependency.resolverPriority =
+              Priority[result.priority];
           }
 
           if (result.invalidateOnEnvChange) {

diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js
@@ -142,6 +142,7 @@ export type Dependency = {|
   customPackageConditions?: Array<string>,
   meta: Meta,
   resolverMeta?: ?Meta,
+  resolverPriority?: $Values<typeof Priority>,
   target: ?Target,
   sourceAssetId: ?string,
   sourcePath: ?ProjectPath,

diff --git a/packages/core/integration-tests/test/hmr.js b/packages/core/integration-tests/test/hmr.js
@@ -516,15 +516,49 @@ module.hot.dispose((data) => {
     });
 
     it('should work across bundles', async function () {
-      let {reloaded} = await testHMRClient('hmr-dynamic', outputs => {
+      let {reloaded, outputs} = await testHMRClient('hmr-dynamic', outputs => {
         assert.deepEqual(outputs, [3]);
         return {
           'local.js': 'exports.a = 5; exports.b = 5;',
         };
       });
 
-      // assert.deepEqual(outputs, [3, 10]);
-      assert(reloaded); // TODO: this should eventually not reload...
+      assert.deepEqual(outputs, [3, 10]);
+      assert(!reloaded);
+    });
+
+    it('should bubble to parents if child returns additional parents', async function () {
+      let {reloaded, outputs} = await testHMRClient('hmr-parents', outputs => {
+        assert.deepEqual(outputs, ['child 2', 'root']);
+        return {
+          'updated.js': 'exports.a = 3;',
+        };
+      });
+
+      assert.deepEqual(outputs, [
+        'child 2',
+        'root',
+        'child 3',
+        'accept child',
+        'root',
+        'accept root',
+      ]);
+      assert(!reloaded);
+    });
+
+    it('should bubble to parents and reload if they do not accept', async function () {
+      let {reloaded, outputs} = await testHMRClient(
+        'hmr-parents-reload',
+        outputs => {
+          assert.deepEqual(outputs, ['child 2', 'root']);
+          return {
+            'updated.js': 'exports.a = 3;',
+          };
+        },
+      );
+
+      assert.deepEqual(outputs, []);
+      assert(reloaded);
     });
 
     it('should work with urls', async function () {

diff --git a/packages/core/integration-tests/test/integration/hmr-parents-reload/child.js b/packages/core/integration-tests/test/integration/hmr-parents-reload/child.js
@@ -0,0 +1,7 @@
+const updated = require('./updated');
+
+output('child ' + updated.a);
+module.hot.accept(getParents => {
+  output('accept child');
+  return getParents();
+});
diff --git a/packages/core/integration-tests/test/integration/hmr-parents-reload/index.js b/packages/core/integration-tests/test/integration/hmr-parents-reload/index.js
@@ -0,0 +1,3 @@
+require('./middle');
+
+output('root');
diff --git a/packages/core/integration-tests/test/integration/hmr-parents-reload/middle.js b/packages/core/integration-tests/test/integration/hmr-parents-reload/middle.js
@@ -0,0 +1 @@
+require('./child');
diff --git a/packages/core/integration-tests/test/integration/hmr-parents-reload/updated.js b/packages/core/integration-tests/test/integration/hmr-parents-reload/updated.js
@@ -0,0 +1 @@
+exports.a = 2;
diff --git a/packages/core/integration-tests/test/integration/hmr-parents/child.js b/packages/core/integration-tests/test/integration/hmr-parents/child.js
@@ -0,0 +1,7 @@
+const updated = require('./updated');
+
+output('child ' + updated.a);
+module.hot.accept(getParents => {
+  output('accept child');
+  return getParents();
+});
diff --git a/packages/core/integration-tests/test/integration/hmr-parents/index.js b/packages/core/integration-tests/test/integration/hmr-parents/index.js
@@ -0,0 +1,6 @@
+require('./middle');
+
+output('root');
+module.hot.accept(() => {
+  output('accept root');
+});
diff --git a/packages/core/integration-tests/test/integration/hmr-parents/middle.js b/packages/core/integration-tests/test/integration/hmr-parents/middle.js
@@ -0,0 +1 @@
+require('./child');
diff --git a/packages/core/integration-tests/test/integration/hmr-parents/updated.js b/packages/core/integration-tests/test/integration/hmr-parents/updated.js
@@ -0,0 +1 @@
+exports.a = 2;
diff --git a/packages/packagers/js/src/DevPackager.js b/packages/packagers/js/src/DevPackager.js
@@ -112,6 +112,16 @@ export class DevPackager {
           }
         }
 
+        // Add dependencies for parcelRequire calls added by runtimes
+        // so that the HMR runtime can correctly traverse parents.
+        let hmrDeps = asset.meta.hmrDeps;
+        if (this.options.hmrOptions && Array.isArray(hmrDeps)) {
+          for (let id of hmrDeps) {
+            invariant(typeof id === 'string');
+            deps[id] = id;
+          }
+        }
+
         let {code, mapBuffer} = results[i];
         let output = code || '';
         wrapped +=

diff --git a/packages/runtimes/hmr/src/loaders/hmr-runtime.js b/packages/runtimes/hmr/src/loaders/hmr-runtime.js
@@ -72,6 +72,7 @@ module.bundle.Module = Module;
 module.bundle.hotData = {};
 
 var checkedAssets /*: {|[string]: boolean|} */,
+  disposedAssets /*: {|[string]: boolean|} */,
   assetsToDispose /*: Array<[ParcelRequire, string]> */,
   assetsToAccept /*: Array<[ParcelRequire, string]> */;
 
@@ -134,6 +135,7 @@ if ((!parent || !parent.isParcelRequire) && typeof WebSocket !== 'undefined') {
   // $FlowFixMe
   ws.onmessage = async function (event /*: {data: string, ...} */) {
     checkedAssets = ({} /*: {|[string]: boolean|} */);
+    disposedAssets = ({} /*: {|[string]: boolean|} */);
     assetsToAccept = [];
     assetsToDispose = [];
 
@@ -171,19 +173,10 @@ if ((!parent || !parent.isParcelRequire) && typeof WebSocket !== 'undefined') {
 
         await hmrApplyUpdates(assets);
 
-        // Dispose all old assets.
-        let processedAssets = ({} /*: {|[string]: boolean|} */);
-        for (let i = 0; i < assetsToDispose.length; i++) {
-          let id = assetsToDispose[i][1];
-
-          if (!processedAssets[id]) {
-            hmrDispose(assetsToDispose[i][0], id);
-            processedAssets[id] = true;
-          }
-        }
+        hmrDisposeQueue();
 
         // Run accept callbacks. This will also re-execute other disposed assets in topological order.
-        processedAssets = {};
+        let processedAssets = {};
         for (let i = 0; i < assetsToAccept.length; i++) {
           let id = assetsToAccept[i][1];
 
@@ -593,6 +586,20 @@ function hmrAcceptCheckOne(
   }
 }
 
+function hmrDisposeQueue() {
+  // Dispose all old assets.
+  for (let i = 0; i < assetsToDispose.length; i++) {
+    let id = assetsToDispose[i][1];
+
+    if (!disposedAssets[id]) {
+      hmrDispose(assetsToDispose[i][0], id);
+      disposedAssets[id] = true;
+    }
+  }
+
+  assetsToDispose = [];
+}
+
 function hmrDispose(bundle /*: ParcelRequire */, id /*: string */) {
   var cached = bundle.cache[id];
   bundle.hotData[id] = {};
@@ -616,18 +623,26 @@ function hmrAccept(bundle /*: ParcelRequire */, id /*: string */) {
   // Run the accept callbacks in the new version of the module.
   var cached = bundle.cache[id];
   if (cached && cached.hot && cached.hot._acceptCallbacks.length) {
+    let assetsToAlsoAccept = [];
     cached.hot._acceptCallbacks.forEach(function (cb) {
-      var assetsToAlsoAccept = cb(function () {
+      let additionalAssets = cb(function () {
         return getParents(module.bundle.root, id);
       });
-      if (assetsToAlsoAccept && assetsToAccept.length) {
-        assetsToAlsoAccept.forEach(function (a) {
-          hmrDispose(a[0], a[1]);
-        });
-
-        // $FlowFixMe[method-unbinding]
-        assetsToAccept.push.apply(assetsToAccept, assetsToAlsoAccept);
+      if (Array.isArray(additionalAssets) && additionalAssets.length) {
+        assetsToAlsoAccept.push(...additionalAssets);
       }
     });
+
+    if (assetsToAccept.length) {
+      let handled = assetsToAlsoAccept.every(function (a) {
+        return hmrAcceptCheck(a[0], a[1]);
+      });
+
+      if (!handled) {
+        return fullReload();
+      }
+
+      hmrDisposeQueue();
+    }
   }
 }
diff --git a/packages/runtimes/js/src/JSRuntime.js b/packages/runtimes/js/src/JSRuntime.js
@@ -94,7 +94,7 @@ export default (new Runtime({
           // The linker handles this for scope-hoisting.
           assets.push({
             filePath: __filename,
-            code: `module.exports = Promise.resolve(module.bundle.root(${JSON.stringify(
+            code: `module.exports = Promise.resolve(parcelRequire(${JSON.stringify(
               bundleGraph.getAssetPublicId(resolved.value),
             )}))`,
             dependency,
@@ -422,10 +422,7 @@ function getLoaderRuntime({
   }
 
   if (mainBundle.type === 'js') {
-    let parcelRequire = bundle.env.shouldScopeHoist
-      ? 'parcelRequire'
-      : 'module.bundle.root';
-    loaderCode += `.then(() => ${parcelRequire}('${bundleGraph.getAssetPublicId(
+    loaderCode += `.then(() => parcelRequire('${bundleGraph.getAssetPublicId(
       bundleGraph.getAssetById(bundleGroup.entryAssetId),
     )}'))`;
   }
@@ -552,7 +549,7 @@ function getURLRuntime(
   to: NamedBundle,
   options: PluginOptions,
 ): RuntimeAsset {
-  let relativePathExpr = getRelativePathExpr(from, to, options);
+  let relativePathExpr = getRelativePathExpr(from, to, options, true);
   let code;
 
   if (dependency.meta.webworker === true && !from.env.isLibrary) {
@@ -633,10 +630,11 @@ function getRelativePathExpr(
   from: NamedBundle,
   to: NamedBundle,
   options: PluginOptions,
+  isURL = to.type !== 'js',
 ): string {
   let relativePath = relativeBundlePath(from, to, {leadingDotSlash: false});
   let res = JSON.stringify(relativePath);
-  if (options.hmrOptions) {
+  if (isURL && options.hmrOptions) {
     res += ' + "?" + Date.now()';
   }
 

diff --git a/packages/transformers/js/core/src/dependency_collector.rs b/packages/transformers/js/core/src/dependency_collector.rs
@@ -12,7 +12,7 @@ use swc_core::{
   ecma::{
     ast::{self, Callee, IdentName, MemberProp},
     atoms::{js_word, JsWord},
-    utils::stack_size::maybe_grow_default,
+    utils::{member_expr, stack_size::maybe_grow_default},
     visit::{Fold, FoldWith},
   },
 };
@@ -93,6 +93,8 @@ pub enum DependencyKind {
   ///
   /// * https://parceljs.org/features/node-emulation/#inlining-fs.readfilesync
   File,
+  /// `parcelRequire` call.
+  Id,
 }
 
 impl fmt::Display for DependencyKind {
@@ -516,6 +518,31 @@ impl<'a> Fold for DependencyCollector<'a> {
                 ))));
                 return call;
               }
+              "parcelRequire" => {
+                if let Some(ast::ExprOrSpread { expr, .. }) = node.args.first() {
+                  if let Some((id, span)) = match_str(expr) {
+                    self.items.push(DependencyDescriptor {
+                      kind: DependencyKind::Id,
+                      loc: SourceLocation::from(&self.source_map, span),
+                      specifier: id,
+                      attributes: None,
+                      is_optional: false,
+                      is_helper: false,
+                      source_type: None,
+                      placeholder: None,
+                    });
+                  }
+                }
+                let mut call = node.fold_children_with(self);
+                if !self.config.scope_hoist {
+                  call.callee = ast::Callee::Expr(Box::new(ast::Expr::Member(member_expr!(
+                    Default::default(),
+                    call.span,
+                    module.bundle.root
+                  ))));
+                }
+                return call;
+              }
               _ => return node.fold_children_with(self),
             }
           }

diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js
@@ -14,6 +14,7 @@ import {transform, transformAsync} from '@parcel/rust';
 import browserslist from 'browserslist';
 import semver from 'semver';
 import nullthrows from 'nullthrows';
+import invariant from 'assert';
 import ThrowableDiagnostic, {
   encodeJSONKeyComponent,
   convertSourceLocationToHighlight,
@@ -756,6 +757,13 @@ export default (new Transformer({
         });
       } else if (dep.kind === 'File') {
         asset.invalidateOnFileChange(dep.specifier);
+      } else if (dep.kind === 'Id') {
+        // Record parcelRequire calls so that the dev packager can add them as dependencies.
+        // This allows the HMR runtime to collect parents across async boundaries (through runtimes).
+        // TODO: ideally this would result as an actual dep in the graph rather than asset.meta.
+        asset.meta.hmrDeps ??= [];
+        invariant(Array.isArray(asset.meta.hmrDeps));
+        asset.meta.hmrDeps.push(dep.specifier);
       } else {
         let meta: JSONObject = {kind: dep.kind};
         if (dep.attributes) {