Mark '*' as used when the reexport is only decided at runtime (#7049)

Author: mischnic

packages/core/core/src/requests/AssetGraphRequest.js

@@ -24,6 +24,7 @@ import invariant from 'assert';
 import nullthrows from 'nullthrows';
 import {PromiseQueue} from '@parcel/utils';
 import {hashString} from '@parcel/hash';
+import logger from '@parcel/logger';
 import ThrowableDiagnostic, {md} from '@parcel/diagnostic';
 import {BundleBehavior, Priority} from '../types';
 import AssetGraph from '../AssetGraph';
@@ -401,6 +402,26 @@ export class AssetGraphBuilder {
       }
     });
 
+    const logFallbackNamespaceInsertion = (
+      assetNode,
+      symbol,
+      depNode1,
+      depNode2,
+    ) => {
+      if (this.options.logLevel === 'verbose') {
+        logger.warn({
+          message: `${fromProjectPathRelative(
+            assetNode.value.filePath,
+          )} reexports "${symbol}", which could be resolved either to the dependency "${
+            depNode1.value.specifier
+          }" or "${
+            depNode2.value.specifier
+          }" at runtime. Adding a namespace object to fall back on.`,
+          origin: '@parcel/core',
+        });
+      }
+    };
+
     // Because namespace reexports introduce ambiguity, go up the graph from the leaves to the
     // root and remove requested symbols that aren't actually exported
     this.propagateSymbolsUp((assetNode, incomingDeps, outgoingDeps) => {
@@ -424,7 +445,9 @@ export class AssetGraphBuilder {
         }
       }
 
-      let reexportedSymbols = new Set<Symbol>();
+      // the symbols that are reexport (not used in `asset`) -> the corresponding outgoingDep(s)
+      // There could be multiple dependencies with non-statically analyzable exports
+      let reexportedSymbols = new Map<Symbol, DependencyNode>();
       for (let outgoingDep of outgoingDeps) {
         let outgoingDepSymbols = outgoingDep.value.symbols;
         if (!outgoingDepSymbols) continue;
@@ -441,7 +464,20 @@ export class AssetGraphBuilder {
         }
 
         if (outgoingDepSymbols.get('*')?.local === '*') {
-          outgoingDep.usedSymbolsUp.forEach(s => reexportedSymbols.add(s));
+          outgoingDep.usedSymbolsUp.forEach(s => {
+            // If the symbol could come from multiple assets at runtime, assetNode's
+            // namespace will be needed at runtime to perform the lookup on.
+            if (reexportedSymbols.has(s) && !assetNode.usedSymbols.has('*')) {
+              logFallbackNamespaceInsertion(
+                assetNode,
+                s,
+                nullthrows(reexportedSymbols.get(s)),
+                outgoingDep,
+              );
+              assetNode.usedSymbols.add('*');
+            }
+            reexportedSymbols.set(s, outgoingDep);
+          });
         }
 
         for (let s of outgoingDep.usedSymbolsUp) {
@@ -458,7 +494,19 @@ export class AssetGraphBuilder {
 
           let reexported = assetSymbolsInverse?.get(local);
           if (reexported != null) {
-            reexported.forEach(s => reexportedSymbols.add(s));
+            reexported.forEach(s => {
+              // see same code above
+              if (reexportedSymbols.has(s) && !assetNode.usedSymbols.has('*')) {
+                logFallbackNamespaceInsertion(
+                  assetNode,
+                  s,
+                  nullthrows(reexportedSymbols.get(s)),
+                  outgoingDep,
+                );
+                assetNode.usedSymbols.add('*');
+              }
+              reexportedSymbols.set(s, outgoingDep);
+            });
           }
         }
       }

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-1/empty.js

@@ -0,0 +1,2 @@
+import { mergeWith } from "./lib";
+output = mergeWith;

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-1/index.js

@@ -0,0 +1,2 @@
+export * from "./other";
+export * from "./empty";

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-1/lib.js

@@ -0,0 +1 @@
+module.exports = 2;

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-1/mergeWith.js

@@ -0,0 +1 @@
+export { default as mergeWith } from "./mergeWith";

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-1/other.js

@@ -0,0 +1 @@
+module.exports = { a: 1 };

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-2/a.js

@@ -0,0 +1 @@
+module.exports = { b: 2 };

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-2/b.js

@@ -0,0 +1,3 @@
+import { b } from "./lib";
+
+output = b;

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-2/index.js

@@ -0,0 +1,2 @@
+export * from "./a";
+export * from "./b";

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-2/lib.js

@@ -0,0 +1,3 @@
+import { FOO } from './re-exports';
+
+sideEffectNoop(FOO)

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/async.js

@@ -0,0 +1,3 @@
+import {doStuff} from './stuff';
+
+output = doStuff();

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/entry.js

@@ -0,0 +1,3 @@
+//@flow
+
+export const FOO = 'FOO';

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/foo.js

@@ -0,0 +1,9 @@
+//@flow
+
+import {FOO, BAR, type MyString} from "../re-exports";
+
+const myFunc = (): MyString => FOO + BAR;
+
+let res = myFunc();
+
+export const fooBar = (): MyString => res;

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/other/index.js

@@ -0,0 +1,8 @@
+{
+  "dependencies": {
+    "flow-bin": "0.157.0"
+  },
+  "sideEffects": [
+    "entry.js"
+  ]
+}

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/package.json

@@ -0,0 +1,4 @@
+//@flow
+
+export * from './foo';
+export * from './rest';

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/re-exports.js

@@ -0,0 +1,3 @@
+//@flow
+
+export const BAR = 'BAR';

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/rest/bar.js

@@ -0,0 +1,3 @@
+//@flow
+
+export type Hello = 1 | 2;

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/rest/hello.js

@@ -0,0 +1,5 @@
+//@flow
+
+export * from './bar';
+export * from './myString';
+export * from './hello';

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/rest/index.js

@@ -0,0 +1,3 @@
+//@flow
+
+export type MyString = string;

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/rest/myString.js

@@ -0,0 +1,11 @@
+//@flow
+
+import {fooBar} from './other'
+import {FOO, BAR, type MyString} from './re-exports'
+import("./async");
+
+const res = (): MyString => fooBar() + "!";
+// $FlowFixMe
+sideEffectNoop(res());
+
+export const doStuff = res;

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/stuff.js

@@ -0,0 +1 @@
+module.exports = { a: 1 };

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-3/yarn.lock

@@ -0,0 +1 @@
+module.exports = { b: 2 };

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-nested/a.js

@@ -0,0 +1 @@
+module.exports = { c: 3 };

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-nested/b.js

@@ -0,0 +1 @@
+module.exports = { d: 4 };

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-nested/c.js

@@ -0,0 +1,4 @@
+import { b } from "./lib";
+import { d } from "./lib2";
+
+output = `${b} ${d}`

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-nested/d.js

@@ -0,0 +1,2 @@
+export * from "./lib1";
+export * from "./lib2";

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-nested/index.js

@@ -0,0 +1,2 @@
+export * from "./a";
+export * from "./b";

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-nested/lib.js

@@ -0,0 +1,2 @@
+export * from "./c";
+export * from "./d";

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-nested/lib1.js

@@ -261,6 +261,53 @@ describe('scope hoisting', function() {
       assert.equal(output, 6);
     });
 
+    it('supports re-exporting all when falling back to namespace at runtime 1', async function() {
+      let b = await bundle(
+        path.join(
+          __dirname,
+          '/integration/scope-hoisting/es6/re-export-all-fallback-1/index.js',
+        ),
+      );
+
+      let output = await run(b);
+      assert.strictEqual(output, 2);
+    });
+
+    it('supports re-exporting all when falling back to namespace at runtime 2', async function() {
+      let b = await bundle(
+        path.join(
+          __dirname,
+          '/integration/scope-hoisting/es6/re-export-all-fallback-2/index.js',
+        ),
+      );
+
+      let output = await run(b);
+      assert.strictEqual(output, 2);
+    });
+
+    it('supports re-exporting all when falling back to namespace at runtime 3', async function() {
+      let b = await bundle(
+        path.join(
+          __dirname,
+          'integration/scope-hoisting/es6/re-export-all-fallback-3/entry.js',
+        ),
+      );
+      let output = await run(b);
+      assert.strictEqual(output, 'FOOBAR!');
+    });
+
+    it('supports nested re-exporting all when falling back to namespace at runtime', async function() {
+      let b = await bundle(
+        path.join(
+          __dirname,
+          '/integration/scope-hoisting/es6/re-export-all-fallback-nested/index.js',
+        ),
+      );
+
+      let output = await run(b);
+      assert.strictEqual(output, '2 4');
+    });
+
     it('supports re-exporting all exports from an external module', async function() {
       let b = await bundle(
         path.join(
@@ -3293,7 +3340,7 @@ describe('scope hoisting', function() {
           },
         });
 
-        assert.deepEqual(calls, ['esm', 'commonjs']);
+        assert.deepEqual(calls, ['esm', 'commonjs', 'index']);
         assert.deepEqual(output, 'Message 1');
       });
 

packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-all-fallback-nested/lib2.js

@@ -978,7 +978,10 @@ ${code}
             if (
               isWrapped ||
               resolved.meta.staticExports === false ||
-              nullthrows(this.bundleGraph.getUsedSymbols(resolved)).has('*')
+              nullthrows(this.bundleGraph.getUsedSymbols(resolved)).has('*') ||
+              // an empty asset
+              (!resolved.meta.hasCJSExports &&
+                resolved.symbols.hasExportSymbol('*'))
             ) {
               let obj = this.getSymbolResolution(asset, resolved, '*', dep);
               append += `$parcel$exportWildcard($${assetId}$exports, ${obj});\n`;
@@ -1007,6 +1010,7 @@ ${code}
                 prepend += `$parcel$export($${assetId}$exports, ${JSON.stringify(
                   symbol,
                 )}, ${get}${set});\n`;
+                this.usedHelpers.add('$parcel$export');
                 prependLineCount++;
               }
             }