[Symbol Propagation] Non-deterministic bundle hashes (#8212)

parcel-bundler · Jul 27, 2022 · ad5b777 · ad5b777
1 parent 411d4c2
commit ad5b777
Show file tree

Hide file tree

Showing 14 changed files with 151 additions and 2 deletions.
diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js
@@ -260,6 +260,10 @@ export class AssetGraphBuilder {
   }
 
   propagateSymbols() {
+    // Keep track of dependencies that have changes to their used symbols,
+    // so we can sort them after propagation.
+    let changedDeps = new Set<DependencyNode>();
+
     // Propagate the requested symbols down from the root to the leaves
     this.propagateSymbolsDown((assetNode, incomingDeps, outgoingDeps) => {
       if (!assetNode.value.symbols) return;
@@ -568,6 +572,7 @@ export class AssetGraphBuilder {
         }
 
         if (!equalSet(incomingDepUsedSymbolsUpOld, incomingDep.usedSymbolsUp)) {
+          changedDeps.add(incomingDep);
           incomingDep.usedSymbolsUpDirtyUp = true;
         }
 
@@ -595,6 +600,12 @@ export class AssetGraphBuilder {
       }
       return errors;
     });
+    // Sort usedSymbolsUp so they are a consistent order across builds.
+    // This ensures a consistent ordering of these symbols when packaging.
+    // See https://github.com/parcel-bundler/parcel/pull/8212
+    for (let dep of changedDeps) {
+      dep.usedSymbolsUp = new Set([...dep.usedSymbolsUp].sort());
+    }
   }
 
   propagateSymbolsDown(

diff --git a/...tion-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/index.html b/...tion-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/index.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<html>
+<body>
+  <script type="module" src="./index.js"></script>
+</body>
+</html>
diff --git a/...ration-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/index.js b/...ration-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/index.js
@@ -0,0 +1,7 @@
+import {foo,bar} from './library';
+
+function other() {
+  return foo+bar;
+}
+
+export {other};
diff --git a/...-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/library/bar.js b/...-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/library/bar.js
@@ -0,0 +1,7 @@
+import {baz, bag} from '../utils';
+
+function bar() {
+  return baz+bag;
+}
+
+export {bar};
diff --git a/...-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/library/foo.js b/...-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/library/foo.js
@@ -0,0 +1,7 @@
+import {bag} from '../utils';
+
+function foo() {
+  return bag;
+}
+
+export {foo};
diff --git a/...ests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/library/index.js b/...ests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/library/index.js
@@ -0,0 +1,2 @@
+export {foo} from './foo';
+export {bar} from './bar';
diff --git a/.../test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/library/package.json b/.../test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/library/package.json
@@ -0,0 +1,3 @@
+{
+  "sideEffects": false
+}
diff --git a/...on-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/utils/bag.js b/...on-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/utils/bag.js
@@ -0,0 +1,7 @@
+export function bag() {
+  return 'bag';
+};
+
+export function baz() {
+  return bag() + 'baz';
+}
diff --git a/...-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/utils/empty.js b/...-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/utils/empty.js
diff --git a/...-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/utils/index.js b/...-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/utils/index.js
@@ -0,0 +1,2 @@
+export * from './bag';
+export * from './empty';
diff --git a/...ts/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/utils/package.json b/...ts/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/utils/package.json
@@ -0,0 +1,3 @@
+{
+  "sideEffects": false
+}
diff --git a/...ation-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/yarn.lock b/...ation-tests/test/integration/scope-hoisting/es6/non-deterministic-bundle-hashes/yarn.lock
diff --git a/packages/core/integration-tests/test/react-refresh.js b/packages/core/integration-tests/test/react-refresh.js
@@ -37,10 +37,14 @@ if (MessageChannel) {
 
       let b,
         root,
-        randoms = {};
+        randoms,
+        subscription,
+        window = {};
 
       beforeEach(async () => {
-        ({b, root, randoms} = await setup(path.join(testDir, 'index.html')));
+        ({b, root, randoms, subscription, window} = await setup(
+          path.join(testDir, 'index.html'),
+        ));
       });
 
       it('retains state in functional components', async function () {
@@ -62,6 +66,10 @@ if (MessageChannel) {
         assert.equal(randoms.fooNum, fooNum);
         assert.equal(fooText, 'OtherFunctional');
       });
+
+      afterEach(async () => {
+        await cleanup({subscription, window});
+      });
     });
 
     describe('synchronous', () => {

diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js
@@ -2,6 +2,7 @@ import assert from 'assert';
 import path from 'path';
 import nullthrows from 'nullthrows';
 import {normalizePath} from '@parcel/utils';
+import {createWorkerFarm} from '@parcel/core';
 import {md} from '@parcel/diagnostic';
 import {
   assertBundles,
@@ -5532,4 +5533,89 @@ describe('scope hoisting', function () {
     let output = await run(b);
     assert.strictEqual(output, 'foo');
   });
+
+  it('produce the same bundle hash regardless of transformation order', async function () {
+    let testDir = path.join(
+      __dirname,
+      'integration/scope-hoisting/es6/non-deterministic-bundle-hashes',
+    );
+
+    const waitHandler = (fileToDelay, fileToWaitFor) => {
+      const waitMap = new Map();
+
+      function wait(filePath) {
+        if (waitMap.has(filePath)) {
+          return Promise.resolve();
+        }
+        return new Promise(resolve => {
+          waitMap.set(filePath, resolve);
+        });
+      }
+      // a set of filepaths that have been read
+      function seen(filePath) {
+        // check map of things we're waiting for to resolved promises
+        let promisesToResolve = waitMap.get(filePath);
+        if (promisesToResolve) {
+          // if we find any, we call it
+          promisesToResolve();
+        }
+        waitMap.set(filePath, null);
+      }
+
+      return {
+        get(target, prop) {
+          let original = Reflect.get(...arguments);
+          if (prop === 'readFile') {
+            return async function (...args) {
+              if (args[0].includes(fileToDelay)) {
+                await wait(fileToWaitFor);
+              }
+              let result = await original.apply(this, args);
+              seen(path.basename(args[0]));
+              return result;
+            };
+          }
+          return original;
+        },
+      };
+    };
+
+    let workerFarm = createWorkerFarm({
+      maxConcurrentWorkers: 0,
+    });
+
+    let slowFooFS = new Proxy(overlayFS, waitHandler('foo.js', 'bar.js'));
+
+    try {
+      let b = await bundle(path.join(testDir, 'index.html'), {
+        inputFS: slowFooFS,
+        outputFS: slowFooFS,
+        shouldDisableCache: true,
+        workerFarm,
+      });
+
+      let bundleHashDelayFoo = b
+        .getBundles()
+        .find(b => b.filePath.endsWith('.js') && b.filePath.includes('index'))
+        .filePath.split('.')[1];
+
+      let slowBarFS = new Proxy(overlayFS, waitHandler('bar.js', 'foo.js'));
+
+      let b2 = await bundle(path.join(testDir, 'index.html'), {
+        inputFS: slowBarFS,
+        outputFS: slowBarFS,
+        shouldDisableCache: true,
+        workerFarm,
+      });
+
+      let bundleHashDelayBar = b2
+        .getBundles()
+        .find(b => b.filePath.endsWith('.js') && b.filePath.includes('index'))
+        .filePath.split('.')[1];
+
+      assert.strictEqual(bundleHashDelayFoo, bundleHashDelayBar);
+    } finally {
+      await workerFarm.end();
+    }
+  });
 });
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		export {foo} from './foo';
		export {bar} from './bar';
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		export * from './bag';
		export * from './empty';