Apply HMR updates in topological order (#8752)

parcel-bundler · Jan 7, 2023 · fdae6c0 · fdae6c0
1 parent e21af59
commit fdae6c0
Show file tree

Hide file tree

Showing 9 changed files with 160 additions and 43 deletions.
diff --git a/packages/core/integration-tests/test/hmr.js b/packages/core/integration-tests/test/hmr.js
@@ -447,10 +447,10 @@ module.hot.dispose((data) => {
         ['eval:local', 1, null],
         ['eval:index', 1, null],
         ['dispose:other', 1],
-        ['eval:other', 3, {value: 1}],
         ['dispose:local', 1],
-        ['eval:local', 3, {value: 1}],
         ['dispose:index', 1],
+        ['eval:other', 3, {value: 1}],
+        ['eval:local', 3, {value: 1}],
         ['eval:index', 3, {value: 1}],
       ]);
     });

diff --git a/packages/core/integration-tests/test/integration/react-refresh-circular/App.js b/packages/core/integration-tests/test/integration/react-refresh-circular/App.js
@@ -0,0 +1,6 @@
+import React from 'react';
+import {Provider} from './Provider';
+
+export function App() {
+  return <Provider />
+}
diff --git a/packages/core/integration-tests/test/integration/react-refresh-circular/Consumer.js b/packages/core/integration-tests/test/integration/react-refresh-circular/Consumer.js
@@ -0,0 +1,10 @@
+import {useContext} from 'react';
+import {Context} from './Provider';
+
+// This prevents the module from being self accepting
+// (not all exports are react components).
+export function tmp() {}
+
+export function Consumer() {
+  return <>{String(useContext(Context))}</>;
+}
diff --git a/packages/core/integration-tests/test/integration/react-refresh-circular/Provider.js b/packages/core/integration-tests/test/integration/react-refresh-circular/Provider.js
@@ -0,0 +1,14 @@
+import React from 'react';
+import {Consumer} from './Consumer';
+
+// This prevents the module from being self accepting
+// since it is not a react component.
+export let Context = React.createContext(null);
+
+export function Provider() {
+  return (
+    <Context.Provider value={2}>
+      <Consumer />
+    </Context.Provider>
+  );
+}
diff --git a/packages/core/integration-tests/test/integration/react-refresh-circular/index.html b/packages/core/integration-tests/test/integration/react-refresh-circular/index.html
@@ -0,0 +1,2 @@
+<div id="root"></div>
+<script type="module" src="index.js"></script>
diff --git a/packages/core/integration-tests/test/integration/react-refresh-circular/index.js b/packages/core/integration-tests/test/integration/react-refresh-circular/index.js
@@ -0,0 +1,6 @@
+import ReactDOM from "react-dom";
+import { App } from "./App";
+import { act } from "react-dom/test-utils";
+
+export default () =>
+  act(async () => {ReactDOM.render(<App />, document.getElementById("root"));});
diff --git a/packages/core/integration-tests/test/integration/react-refresh-circular/package.json b/packages/core/integration-tests/test/integration/react-refresh-circular/package.json
@@ -0,0 +1,6 @@
+{
+    "dependencies": {
+        "react": "^17.0.0",
+        "react-dom": "^17.0.0"
+    }
+}
diff --git a/packages/core/integration-tests/test/react-refresh.js b/packages/core/integration-tests/test/react-refresh.js
@@ -61,9 +61,9 @@ if (MessageChannel) {
         let [, indexNum, appNum, fooText, fooNum] = root.textContent.match(
           /^([\d.]+) ([\d.]+) ([\w]+):([\d.]+)$/,
         );
-        assert.equal(randoms.indexNum, indexNum);
-        assert.equal(randoms.appNum, appNum);
-        assert.equal(randoms.fooNum, fooNum);
+        assert.equal(randoms?.indexNum, indexNum);
+        assert.equal(randoms?.appNum, appNum);
+        assert.equal(randoms?.fooNum, fooNum);
         assert.equal(fooText, 'OtherFunctional');
       });
 
@@ -101,9 +101,9 @@ if (MessageChannel) {
         let [, indexNum, appNum, fooText, fooNum] = root.textContent.match(
           /^([\d.]+) ([\d.]+) ([\w]+):([\d.]+)$/,
         );
-        assert.equal(randoms.indexNum, indexNum);
-        assert.equal(randoms.appNum, appNum);
-        assert.equal(randoms.fooNum, fooNum);
+        assert.equal(randoms?.indexNum, indexNum);
+        assert.equal(randoms?.appNum, appNum);
+        assert.equal(randoms?.fooNum, fooNum);
         assert.equal(fooText, 'OtherFunctional');
       });
 
@@ -122,9 +122,9 @@ if (MessageChannel) {
           root.textContent.match(
             /^([\d.]+) ([\d.]+) ([\w]+):([\d.]+):([\d.]+)$/,
           );
-        assert.equal(randoms.indexNum, indexNum);
-        assert.equal(randoms.appNum, appNum);
-        assert.notEqual(randoms.fooNum, fooNum);
+        assert.equal(randoms?.indexNum, indexNum);
+        assert.equal(randoms?.appNum, appNum);
+        assert.notEqual(randoms?.fooNum, fooNum);
         assert(fooNum2);
         assert.equal(fooText, 'Hooks');
       });
@@ -143,9 +143,9 @@ if (MessageChannel) {
         let [, indexNum, appNum, fooText, fooNum] = root.textContent.match(
           /^([\d.]+) ([\d.]+) ([\w]+):([\d.]+)$/,
         );
-        assert.equal(randoms.indexNum, indexNum);
-        assert.equal(randoms.appNum, appNum);
-        assert.notEqual(randoms.fooNum, fooNum);
+        assert.equal(randoms?.indexNum, indexNum);
+        assert.equal(randoms?.appNum, appNum);
+        assert.notEqual(randoms?.fooNum, fooNum);
         assert.equal(fooText, 'Class');
       });
 
@@ -173,7 +173,7 @@ if (MessageChannel) {
       });
 
       it('retains state in async components on change', async function () {
-        assert.equal(randoms.fooText, 'Async');
+        assert.equal(randoms?.fooText, 'Async');
 
         await fs.mkdirp(testDir);
         await fs.copyFile(
@@ -188,9 +188,9 @@ if (MessageChannel) {
         let [, indexNum, appNum, fooText, fooNum] = root.textContent.match(
           /^([\d.]+) ([\d.]+) ([\w]+):([\d.]+)$/,
         );
-        assert.equal(randoms.indexNum, indexNum);
-        assert.equal(randoms.appNum, appNum);
-        assert.equal(randoms.fooNum, fooNum);
+        assert.equal(randoms?.indexNum, indexNum);
+        assert.equal(randoms?.appNum, appNum);
+        assert.equal(randoms?.fooNum, fooNum);
         assert.equal(fooText, 'OtherAsync');
       });
 
@@ -199,6 +199,55 @@ if (MessageChannel) {
       });
     });
 
+    describe('circular context dependency', () => {
+      const testDir = path.join(
+        __dirname,
+        '/integration/react-refresh-circular',
+      );
+
+      let b,
+        root,
+        subscription,
+        window = {};
+
+      beforeEach(async () => {
+        ({b, root, subscription, window} = await setup(
+          path.join(testDir, 'index.html'),
+        ));
+      });
+
+      it('does not become null when modifying provider', async function () {
+        await fs.mkdirp(testDir);
+        let f = path.join(testDir, 'Provider.js');
+        await fs.writeFile(f, (await fs.readFile(f, 'utf8')).replace('2', '3'));
+        assert.equal((await getNextBuild(b)).type, 'buildSuccess');
+
+        // Wait for the hmr-runtime to process the event
+        await sleep(100);
+
+        assert.equal(root.textContent, '3');
+      });
+
+      it('does not become null when modifying consumer', async function () {
+        await fs.mkdirp(testDir);
+        let f = path.join(testDir, 'Consumer.js');
+        await fs.writeFile(
+          f,
+          (await fs.readFile(f, 'utf8')).replace('tmp', 'foo'),
+        );
+        assert.equal((await getNextBuild(b)).type, 'buildSuccess');
+
+        // Wait for the hmr-runtime to process the event
+        await sleep(100);
+
+        assert.equal(root.textContent, '2');
+      });
+
+      afterEach(async () => {
+        await cleanup({subscription, window});
+      });
+    });
+
     it('does not error on inline scripts', async () => {
       let port = await getPort();
       let b = await bundle(
@@ -314,15 +363,15 @@ async function setup(entry) {
   ).default();
   await sleep(100);
 
-  let [, indexNum, appNum, fooText, fooNum] = root.textContent.match(
-    /^([\d.]+) ([\d.]+) ([\w]+):([\d.]+)$/,
-  );
-  assert(indexNum);
-  assert(appNum);
-  assert(fooNum);
-
-  randoms = {indexNum, appNum, fooText, fooNum};
+  let m = root.textContent.match(/^([\d.]+) ([\d.]+) ([\w]+):([\d.]+)$/);
+  if (m) {
+    let [, indexNum, appNum, fooText, fooNum] = m;
+    assert(indexNum);
+    assert(appNum);
+    assert(fooNum);
 
+    randoms = {indexNum, appNum, fooText, fooNum};
+  }
   return {port, b, window, randoms, subscription, root};
 }
 

diff --git a/packages/runtimes/hmr/src/loaders/hmr-runtime.js b/packages/runtimes/hmr/src/loaders/hmr-runtime.js
@@ -9,7 +9,7 @@ import type {
 interface ParcelRequire {
   (string): mixed;
   cache: {|[string]: ParcelModule|};
-  hotData: mixed;
+  hotData: {|[string]: mixed|};
   Module: any;
   parent: ?ParcelRequire;
   isParcelRequire: true;
@@ -55,7 +55,7 @@ var OldModule = module.bundle.Module;
 function Module(moduleName) {
   OldModule.call(this, moduleName);
   this.hot = {
-    data: module.bundle.hotData,
+    data: module.bundle.hotData[moduleName],
     _acceptCallbacks: [],
     _disposeCallbacks: [],
     accept: function (fn) {
@@ -65,12 +65,13 @@ function Module(moduleName) {
       this._disposeCallbacks.push(fn);
     },
   };
-  module.bundle.hotData = undefined;
+  module.bundle.hotData[moduleName] = undefined;
 }
 module.bundle.Module = Module;
+module.bundle.hotData = {};
 
 var checkedAssets /*: {|[string]: boolean|} */,
-  acceptedAssets /*: {|[string]: boolean|} */,
+  assetsToDispose /*: Array<[ParcelRequire, string]> */,
   assetsToAccept /*: Array<[ParcelRequire, string]> */;
 
 function getHostname() {
@@ -119,8 +120,8 @@ if ((!parent || !parent.isParcelRequire) && typeof WebSocket !== 'undefined') {
   // $FlowFixMe
   ws.onmessage = async function (event /*: {data: string, ...} */) {
     checkedAssets = ({} /*: {|[string]: boolean|} */);
-    acceptedAssets = ({} /*: {|[string]: boolean|} */);
     assetsToAccept = [];
+    assetsToDispose = [];
 
     var data /*: HMRMessage */ = JSON.parse(event.data);
 
@@ -154,10 +155,25 @@ if ((!parent || !parent.isParcelRequire) && typeof WebSocket !== 'undefined') {
 
         await hmrApplyUpdates(assets);
 
-        for (var i = 0; i < assetsToAccept.length; i++) {
-          var id = assetsToAccept[i][1];
-          if (!acceptedAssets[id]) {
-            hmrAcceptRun(assetsToAccept[i][0], id);
+        // 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;
+          }
+        }
+
+        // Run accept callbacks. This will also re-execute other disposed assets in topological order.
+        processedAssets = {};
+        for (let i = 0; i < assetsToAccept.length; i++) {
+          let id = assetsToAccept[i][1];
+
+          if (!processedAssets[id]) {
+            hmrAccept(assetsToAccept[i][0], id);
+            processedAssets[id] = true;
           }
         }
       } else fullReload();
@@ -553,41 +569,49 @@ function hmrAcceptCheckOne(
   checkedAssets[id] = true;
 
   var cached = bundle.cache[id];
-
-  assetsToAccept.push([bundle, id]);
+  assetsToDispose.push([bundle, id]);
 
   if (!cached || (cached.hot && cached.hot._acceptCallbacks.length)) {
+    assetsToAccept.push([bundle, id]);
     return true;
   }
 }
 
-function hmrAcceptRun(bundle /*: ParcelRequire */, id /*: string */) {
+function hmrDispose(bundle /*: ParcelRequire */, id /*: string */) {
   var cached = bundle.cache[id];
-  bundle.hotData = {};
+  bundle.hotData[id] = {};
   if (cached && cached.hot) {
-    cached.hot.data = bundle.hotData;
+    cached.hot.data = bundle.hotData[id];
   }
 
   if (cached && cached.hot && cached.hot._disposeCallbacks.length) {
     cached.hot._disposeCallbacks.forEach(function (cb) {
-      cb(bundle.hotData);
+      cb(bundle.hotData[id]);
     });
   }
 
   delete bundle.cache[id];
+}
+
+function hmrAccept(bundle /*: ParcelRequire */, id /*: string */) {
+  // Execute the module.
   bundle(id);
 
-  cached = bundle.cache[id];
+  // Run the accept callbacks in the new version of the module.
+  var cached = bundle.cache[id];
   if (cached && cached.hot && cached.hot._acceptCallbacks.length) {
     cached.hot._acceptCallbacks.forEach(function (cb) {
       var assetsToAlsoAccept = 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);
       }
     });
   }
-  acceptedAssets[id] = true;
 }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		<div id="root"></div>
		<script type="module" src="index.js"></script>