From 44f192a3374a0118d3090ad1c955e2fd218f27fe Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 13:26:49 +0200 Subject: [PATCH 01/48] POC --- packages/core/core/src/AssetGraph.js | 8 +- packages/core/core/src/BundleGraph.js | 42 ++++--- packages/core/core/src/dumpGraphToGraphViz.js | 24 +++- .../core/src/requests/AssetGraphRequest.js | 119 ++++++++++++------ packages/core/core/src/types.js | 7 +- 5 files changed, 139 insertions(+), 61 deletions(-) diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index 5afb1f68e1e..2a1f99d6c49 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -55,8 +55,8 @@ export function nodeFromDep(dep: Dependency): DependencyNode { value: dep, deferred: false, excluded: false, - usedSymbolsDown: new Set(), - usedSymbolsUp: new Set(), + usedSymbolsDown: new Map(), + usedSymbolsUp: new Map(), usedSymbolsDownDirty: true, usedSymbolsUpDirtyDown: true, usedSymbolsUpDirtyUp: true, @@ -87,7 +87,7 @@ export function nodeFromAsset(asset: Asset): AssetNode { id: asset.id, type: 'asset', value: asset, - usedSymbols: new Set(), + usedSymbols: new Map(), usedSymbolsDownDirty: true, usedSymbolsUpDirty: true, }; @@ -256,7 +256,7 @@ export default class AssetGraph extends ContentGraph { if (node.value.env.isLibrary) { // in library mode, all of the entry's symbols are "used" - node.usedSymbolsDown.add('*'); + node.usedSymbolsDown.set('*', new Set()); } return node; }); diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index b0ab7d12864..2f30e09da04 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -1737,7 +1737,7 @@ export default class BundleGraph { let node = this._graph.getNodeByContentKey(asset.id); invariant(node && node.type === 'asset'); return this._symbolPropagationRan - ? makeReadOnlySet(node.usedSymbols) + ? makeReadOnlySet(new Set(node.usedSymbols.keys())) : null; } @@ -1745,7 +1745,7 @@ export default class BundleGraph { let node = this._graph.getNodeByContentKey(dep.id); invariant(node && node.type === 'dependency'); return this._symbolPropagationRan - ? makeReadOnlySet(node.usedSymbolsUp) + ? makeReadOnlySet(new Set(node.usedSymbolsUp.keys())) : null; } @@ -1758,22 +1758,36 @@ export default class BundleGraph { let existingNode = nullthrows(this._graph.getNode(existingNodeId)); // Merge symbols, recompute dep.exluded based on that + + // eslint-disable-next-line no-inner-declarations + function merge( + a: Map>, + b: Map>, + ): Map> { + let result = new Map(a); + for (let [k, v] of b) { + let existing = result.get(k); + result.set(k, existing ? new Set([...existing, ...v]) : v); + } + return result; + } + if (existingNode.type === 'asset') { invariant(otherNode.type === 'asset'); - existingNode.usedSymbols = new Set([ - ...existingNode.usedSymbols, - ...otherNode.usedSymbols, - ]); + existingNode.usedSymbols = merge( + existingNode.usedSymbols, + otherNode.usedSymbols, + ); } else if (existingNode.type === 'dependency') { invariant(otherNode.type === 'dependency'); - existingNode.usedSymbolsDown = new Set([ - ...existingNode.usedSymbolsDown, - ...otherNode.usedSymbolsDown, - ]); - existingNode.usedSymbolsUp = new Set([ - ...existingNode.usedSymbolsUp, - ...otherNode.usedSymbolsUp, - ]); + existingNode.usedSymbolsDown = merge( + existingNode.usedSymbolsDown, + otherNode.usedSymbolsDown, + ); + existingNode.usedSymbolsUp = merge( + existingNode.usedSymbolsUp, + otherNode.usedSymbolsUp, + ); existingNode.excluded = (existingNode.excluded || Boolean(existingNode.hasDeferred)) && diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 79519fa9183..33574a7fae8 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -103,11 +103,22 @@ export default async function dumpGraphToGraphViz( label += '\\nweakSymbols: ' + weakSymbols.join(','); } if (node.usedSymbolsUp.size > 0) { - label += '\\nusedSymbolsUp: ' + [...node.usedSymbolsUp].join(','); + label += + '\\nusedSymbolsUp: ' + + [...node.usedSymbolsUp] + .map(([s, sDeps]) => + sDeps.size > 0 ? `${s}(${[...sDeps].join(',')})` : s, + ) + .join(','); } if (node.usedSymbolsDown.size > 0) { label += - '\\nusedSymbolsDown: ' + [...node.usedSymbolsDown].join(','); + '\\nusedSymbolsDown: ' + + [...node.usedSymbolsDown] + .map(([s, sDeps]) => + sDeps.size > 0 ? `${s}(${[...sDeps].join(',')})` : s, + ) + .join(','); } } else { label += '\\nsymbols: cleared'; @@ -129,7 +140,14 @@ export default async function dumpGraphToGraphViz( .join(';'); } if (node.usedSymbols.size) { - label += '\\nusedSymbols: ' + [...node.usedSymbols].join(','); + label += + '\\nusedSymbols: ' + + [...node.usedSymbols] + + .map(([s, sDeps]) => + sDeps.size > 0 ? `${s}(${[...sDeps].join(',')})` : s, + ) + .join(','); } } else { label += '\\nsymbols: cleared'; diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index e9ebc26898b..2451d19df59 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -1,7 +1,7 @@ // @flow strict-local import type {Diagnostic} from '@parcel/diagnostic'; -import type {NodeId} from '@parcel/graph'; +import type {ContentKey, NodeId} from '@parcel/graph'; import type {Async, Symbol, Meta} from '@parcel/types'; import type {SharedReference} from '@parcel/workers'; import type { @@ -294,15 +294,15 @@ export class AssetGraphBuilder { let isEntry = false; // Used symbols that are exported or reexported (symbol will be removed again later) by asset. - assetNode.usedSymbols = new Set(); + assetNode.usedSymbols = new Map(); // Symbols that have to be namespace reexported by outgoingDeps. - let namespaceReexportedSymbols = new Set(); + let namespaceReexportedSymbols = new Map>(); if (incomingDeps.length === 0) { // Root in the runtimes Graph - assetNode.usedSymbols.add('*'); - namespaceReexportedSymbols.add('*'); + setAddAll(assetNode.usedSymbols, '*', new Set()); + setAddAll(namespaceReexportedSymbols, '*', new Set()); } else { for (let incomingDep of incomingDeps) { if (incomingDep.value.symbols == null) { @@ -310,10 +310,13 @@ export class AssetGraphBuilder { continue; } - for (let exportSymbol of incomingDep.usedSymbolsDown) { + for (let [ + exportSymbol, + exportSymbolDeps, + ] of incomingDep.usedSymbolsDown) { if (exportSymbol === '*') { - assetNode.usedSymbols.add('*'); - namespaceReexportedSymbols.add('*'); + setAddAll(assetNode.usedSymbols, '*', exportSymbolDeps); + setAddAll(namespaceReexportedSymbols, '*', exportSymbolDeps); } if ( !assetSymbols || @@ -321,14 +324,18 @@ export class AssetGraphBuilder { assetSymbols.has('*') ) { // An own symbol or a non-namespace reexport - assetNode.usedSymbols.add(exportSymbol); + setAddAll(assetNode.usedSymbols, exportSymbol, exportSymbolDeps); } // A namespace reexport // (but only if we actually have namespace-exporting outgoing dependencies, // This usually happens with a reexporting asset with many namespace exports which means that // we cannot match up the correct asset with the used symbol at this level.) else if (hasNamespaceOutgoingDeps && exportSymbol !== 'default') { - namespaceReexportedSymbols.add(exportSymbol); + setAddAll( + namespaceReexportedSymbols, + exportSymbol, + exportSymbolDeps, + ); } } } @@ -338,7 +345,7 @@ export class AssetGraphBuilder { // ---------------------------------------------------------- for (let dep of outgoingDeps) { let depUsedSymbolsDownOld = dep.usedSymbolsDown; - let depUsedSymbolsDown = new Set(); + let depUsedSymbolsDown = new Map(); dep.usedSymbolsDown = depUsedSymbolsDown; if ( assetNode.value.sideEffects || @@ -356,9 +363,9 @@ export class AssetGraphBuilder { if (!depSymbols) continue; if (depSymbols.get('*')?.local === '*') { - for (let s of namespaceReexportedSymbols) { + for (let [s, sDeps] of namespaceReexportedSymbols) { // We need to propagate the namespaceReexportedSymbols to all namespace dependencies (= even wrong ones because we don't know yet) - depUsedSymbolsDown.add(s); + setAddAll(depUsedSymbolsDown, s, sDeps); } } @@ -368,15 +375,19 @@ export class AssetGraphBuilder { if (!assetSymbolsInverse || !depSymbols.get(symbol)?.isWeak) { // Bailout or non-weak symbol (= used in the asset itself = not a reexport) - depUsedSymbolsDown.add(symbol); + setAddAll(depUsedSymbolsDown, symbol, new Set([dep.id])); } else { let reexportedExportSymbols = assetSymbolsInverse.get(local); if (reexportedExportSymbols == null) { // not reexported = used in asset itself - depUsedSymbolsDown.add(symbol); + setAddAll(depUsedSymbolsDown, symbol, new Set([dep.id])); } else if (assetNode.usedSymbols.has('*')) { // we need everything - depUsedSymbolsDown.add(symbol); + setAddAll( + depUsedSymbolsDown, + symbol, + nullthrows(assetNode.usedSymbols.get('*')), + ); [...reexportedExportSymbols].forEach(s => assetNode.usedSymbols.delete(s), @@ -387,7 +398,11 @@ export class AssetGraphBuilder { ].filter(s => assetNode.usedSymbols.has(s)); if (usedReexportedExportSymbols.length > 0) { // The symbol is indeed a reexport, so it's not used from the asset itself - depUsedSymbolsDown.add(symbol); + setAddAll( + depUsedSymbolsDown, + symbol, + nullthrows(assetNode.usedSymbols.get(symbol)), + ); usedReexportedExportSymbols.forEach(s => assetNode.usedSymbols.delete(s), @@ -399,7 +414,7 @@ export class AssetGraphBuilder { } else { depUsedSymbolsDown.clear(); } - if (!equalSet(depUsedSymbolsDownOld, depUsedSymbolsDown)) { + if (!equalMapSet(depUsedSymbolsDownOld, depUsedSymbolsDown)) { dep.usedSymbolsDownDirty = true; dep.usedSymbolsUpDirtyDown = true; } @@ -408,7 +423,7 @@ export class AssetGraphBuilder { const logFallbackNamespaceInsertion = ( assetNode, - symbol, + symbol: Symbol, depNode1, depNode2, ) => { @@ -449,9 +464,12 @@ export class AssetGraphBuilder { } } - // 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(); + // the symbols that are reexported (not used in `asset`) -> the original dependencies that requested it + let reexportedSymbols = new Map>(); + // the symbols that are reexported (not used in `asset`) -> the corresponding outgoingDep(s) + // To generate the diagnostic when there are multiple dependencies with non-statically + // analyzable exports + let reexportedSymbolsSource = new Map(); for (let outgoingDep of outgoingDeps) { let outgoingDepSymbols = outgoingDep.value.symbols; if (!outgoingDepSymbols) continue; @@ -462,29 +480,30 @@ export class AssetGraphBuilder { this.assetGraph.getNodeIdByContentKey(outgoingDep.id), ).length === 0 ) { - outgoingDep.usedSymbolsDown.forEach(s => - outgoingDep.usedSymbolsUp.add(s), + outgoingDep.usedSymbolsDown.forEach((sDeps, s) => + setAddAll(outgoingDep.usedSymbolsUp, s, sDeps), ); } if (outgoingDepSymbols.get('*')?.local === '*') { - outgoingDep.usedSymbolsUp.forEach(s => { + outgoingDep.usedSymbolsUp.forEach((sDeps, 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)), + nullthrows(reexportedSymbolsSource.get(s)), outgoingDep, ); - assetNode.usedSymbols.add('*'); + setAddAll(assetNode.usedSymbols, '*', sDeps); } - reexportedSymbols.set(s, outgoingDep); + setAddAll(reexportedSymbols, s, sDeps); + reexportedSymbolsSource.set(s, outgoingDep); }); } - for (let s of outgoingDep.usedSymbolsUp) { + for (let [s, sDeps] of outgoingDep.usedSymbolsUp) { if (!outgoingDep.usedSymbolsDown.has(s)) { // usedSymbolsDown is a superset of usedSymbolsUp continue; @@ -504,12 +523,13 @@ export class AssetGraphBuilder { logFallbackNamespaceInsertion( assetNode, s, - nullthrows(reexportedSymbols.get(s)), + nullthrows(reexportedSymbolsSource.get(s)), outgoingDep, ); - assetNode.usedSymbols.add('*'); + setAddAll(assetNode.usedSymbols, '*', new Set()); } - reexportedSymbols.set(s, outgoingDep); + setAddAll(reexportedSymbols, s, sDeps); + reexportedSymbolsSource.set(s, outgoingDep); }); } } @@ -519,12 +539,12 @@ export class AssetGraphBuilder { for (let incomingDep of incomingDeps) { let incomingDepUsedSymbolsUpOld = incomingDep.usedSymbolsUp; - incomingDep.usedSymbolsUp = new Set(); + incomingDep.usedSymbolsUp = new Map(); let incomingDepSymbols = incomingDep.value.symbols; if (!incomingDepSymbols) continue; let hasNamespaceReexport = incomingDepSymbols.get('*')?.local === '*'; - for (let s of incomingDep.usedSymbolsDown) { + for (let [s, sDeps] of incomingDep.usedSymbolsDown) { if ( assetSymbols == null || // Assume everything could be provided if symbols are cleared assetNode.value.bundleBehavior === BundleBehavior.isolated || @@ -533,7 +553,7 @@ export class AssetGraphBuilder { reexportedSymbols.has(s) || s === '*' ) { - incomingDep.usedSymbolsUp.add(s); + setAddAll(incomingDep.usedSymbolsUp, s, sDeps); } else if (!hasNamespaceReexport) { let loc = incomingDep.value.symbols?.get(s)?.loc; let [resolutionNodeId] = this.assetGraph.getNodeIdsConnectedFrom( @@ -571,7 +591,9 @@ export class AssetGraphBuilder { } } - if (!equalSet(incomingDepUsedSymbolsUpOld, incomingDep.usedSymbolsUp)) { + if ( + !equalMapSet(incomingDepUsedSymbolsUpOld, incomingDep.usedSymbolsUp) + ) { changedDeps.add(incomingDep); incomingDep.usedSymbolsUpDirtyUp = true; } @@ -604,7 +626,9 @@ export class AssetGraphBuilder { // 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()); + dep.usedSymbolsUp = new Map( + [...dep.usedSymbolsUp].sort(([a], [b]) => a.localeCompare(b)), + ); } } @@ -893,6 +917,27 @@ export class AssetGraphBuilder { } } +function equalMapSet( + a: $ReadOnlyMap>, + b: $ReadOnlyMap>, +) { + return ( + a.size === b.size && + [...a].every(([k, v]) => { + let vB = b.get(k); + return vB && equalSet(v, vB); + }) + ); +} function equalSet(a: $ReadOnlySet, b: $ReadOnlySet) { return a.size === b.size && [...a].every(i => b.has(i)); } + +function setAddAll(a: Map>, key: K, b: $ReadOnlySet) { + let target = a.get(key); + if (target == null) { + target = new Set(b); + a.set(key, target); + } + b.forEach(v => nullthrows(target).add(v)); +} diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index 3acd4df21eb..1f5868cd226 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -290,7 +290,7 @@ export type AssetNode = {| id: ContentKey, +type: 'asset', value: Asset, - usedSymbols: Set, + usedSymbols: Map>, hasDeferred?: boolean, usedSymbolsDownDirty: boolean, usedSymbolsUpDirty: boolean, @@ -306,8 +306,9 @@ export type DependencyNode = {| deferred: boolean, /** dependency was deferred (= no used symbols (in immediate parents) & side-effect free) */ hasDeferred?: boolean, - usedSymbolsDown: Set, - usedSymbolsUp: Set, + // symbol -> the dependency that originally requested it + usedSymbolsDown: Map>, + usedSymbolsUp: Map>, /** for the "down" pass, the dependency resolution asset needs to be updated */ usedSymbolsDownDirty: boolean, /** for the "up" pass, the parent asset needs to be updated */ From 3021160368579dada5b6882faece27bb33ae2955 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 15:05:13 +0200 Subject: [PATCH 02/48] f --- packages/core/core/src/AssetGraph.js | 1 + packages/core/core/src/BundleGraph.js | 15 +- packages/core/core/src/dumpGraphToGraphViz.js | 5 +- packages/core/core/src/public/BundleGraph.js | 8 ++ .../core/src/requests/AssetGraphRequest.js | 131 +++++++++++++----- packages/core/core/src/types.js | 6 +- packages/core/types/index.js | 1 + .../packagers/js/src/ScopeHoistingPackager.js | 4 + 8 files changed, 131 insertions(+), 40 deletions(-) diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index 2a1f99d6c49..832162761bd 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -60,6 +60,7 @@ export function nodeFromDep(dep: Dependency): DependencyNode { usedSymbolsDownDirty: true, usedSymbolsUpDirtyDown: true, usedSymbolsUpDirtyUp: true, + symbolTarget: null, }; } diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 2f30e09da04..67f0fd841b7 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -858,6 +858,15 @@ export default class BundleGraph { }); } + getDependenciesWithSymbolTarget(asset: Asset): Array<[Dependency, ?Symbol]> { + let nodeId = this._graph.getNodeIdByContentKey(asset.id); + return this._graph.getNodeIdsConnectedFrom(nodeId).map(id => { + let node = nullthrows(this._graph.getNode(id)); + invariant(node.type === 'dependency'); + return [node.value, node.symbolTarget]; + }); + } + traverseAssets( bundle: Bundle, visit: GraphVisitor, @@ -1402,9 +1411,9 @@ export default class BundleGraph { let found = false; let nonStaticDependency = false; let skipped = false; - let deps = this.getDependencies(asset).reverse(); + let deps = this.getDependenciesWithSymbolTarget(asset).reverse(); let potentialResults = []; - for (let dep of deps) { + for (let [dep, symbolTarget] of deps) { let depSymbols = dep.symbols; if (!depSymbols) { nonStaticDependency = true; @@ -1414,7 +1423,7 @@ export default class BundleGraph { let symbolLookup = new Map( [...depSymbols].map(([key, val]) => [val.local, key]), ); - let depSymbol = symbolLookup.get(identifier); + let depSymbol = symbolLookup.get(symbolTarget ?? identifier); if (depSymbol != null) { let resolved = this.getResolvedAsset(dep); if (!resolved || resolved.id === asset.id) { diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 33574a7fae8..06e4238edaf 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -106,8 +106,9 @@ export default async function dumpGraphToGraphViz( label += '\\nusedSymbolsUp: ' + [...node.usedSymbolsUp] - .map(([s, sDeps]) => - sDeps.size > 0 ? `${s}(${[...sDeps].join(',')})` : s, + .map( + ([s, sAsset]) => + `${s}(${sAsset.asset}.${sAsset.symbol ?? ''})`, ) .join(','); } diff --git a/packages/core/core/src/public/BundleGraph.js b/packages/core/core/src/public/BundleGraph.js index b42a7b68fed..ae6938a474c 100644 --- a/packages/core/core/src/public/BundleGraph.js +++ b/packages/core/core/src/public/BundleGraph.js @@ -236,6 +236,14 @@ export default class BundleGraph }; } + getDependencySymbolTarget(dependency: IDependency): ?Symbol { + let node = nullthrows( + this.#graph._graph.getNodeByContentKey(dependency.id), + ); + invariant(node.type === 'dependency'); + return node.symbolTarget; + } + getExportedSymbols( asset: IAsset, boundary: ?IBundle, diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 2451d19df59..870bb34d7d3 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -398,11 +398,7 @@ export class AssetGraphBuilder { ].filter(s => assetNode.usedSymbols.has(s)); if (usedReexportedExportSymbols.length > 0) { // The symbol is indeed a reexport, so it's not used from the asset itself - setAddAll( - depUsedSymbolsDown, - symbol, - nullthrows(assetNode.usedSymbols.get(symbol)), - ); + setAddAll(depUsedSymbolsDown, symbol, new Set([dep.id])); usedReexportedExportSymbols.forEach(s => assetNode.usedSymbols.delete(s), @@ -441,6 +437,8 @@ export class AssetGraphBuilder { } }; + let replacements = new Map(); + // 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) => { @@ -464,8 +462,11 @@ export class AssetGraphBuilder { } } - // the symbols that are reexported (not used in `asset`) -> the original dependencies that requested it - let reexportedSymbols = new Map>(); + // the symbols that are reexported (not used in `asset`) -> asset they resolved to + let reexportedSymbols = new Map< + Symbol, + {|asset: ContentKey, symbol: ?Symbol|}, + >(); // the symbols that are reexported (not used in `asset`) -> the corresponding outgoingDep(s) // To generate the diagnostic when there are multiple dependencies with non-statically // analyzable exports @@ -480,30 +481,38 @@ export class AssetGraphBuilder { this.assetGraph.getNodeIdByContentKey(outgoingDep.id), ).length === 0 ) { - outgoingDep.usedSymbolsDown.forEach((sDeps, s) => - setAddAll(outgoingDep.usedSymbolsUp, s, sDeps), + outgoingDep.usedSymbolsDown.forEach((_, s) => + outgoingDep.usedSymbolsUp.set(s, {asset: assetNode.id, symbol: s}), ); } if (outgoingDepSymbols.get('*')?.local === '*') { - outgoingDep.usedSymbolsUp.forEach((sDeps, s) => { + outgoingDep.usedSymbolsUp.forEach((sResolved, 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(reexportedSymbolsSource.get(s)), - outgoingDep, + if (reexportedSymbols.has(s)) { + if (!assetNode.usedSymbols.has('*')) { + logFallbackNamespaceInsertion( + assetNode, + s, + nullthrows(reexportedSymbolsSource.get(s)), + outgoingDep, + ); + } + setAddAll( + assetNode.usedSymbols, + '*', + nullthrows(outgoingDep.usedSymbolsDown.get(s)), ); - setAddAll(assetNode.usedSymbols, '*', sDeps); + reexportedSymbols.set(s, {asset: assetNode.id, symbol: s}); + } else { + reexportedSymbols.set(s, sResolved); + reexportedSymbolsSource.set(s, outgoingDep); } - setAddAll(reexportedSymbols, s, sDeps); - reexportedSymbolsSource.set(s, outgoingDep); }); } - for (let [s, sDeps] of outgoingDep.usedSymbolsUp) { + for (let [s, sResolved] of outgoingDep.usedSymbolsUp) { if (!outgoingDep.usedSymbolsDown.has(s)) { // usedSymbolsDown is a superset of usedSymbolsUp continue; @@ -519,18 +528,34 @@ export class AssetGraphBuilder { if (reexported != null) { reexported.forEach(s => { // see same code above - if (reexportedSymbols.has(s) && !assetNode.usedSymbols.has('*')) { - logFallbackNamespaceInsertion( - assetNode, - s, - nullthrows(reexportedSymbolsSource.get(s)), - outgoingDep, + if (reexportedSymbols.has(s)) { + if (!assetNode.usedSymbols.has('*')) { + logFallbackNamespaceInsertion( + assetNode, + s, + nullthrows(reexportedSymbolsSource.get(s)), + outgoingDep, + ); + } + setAddAll( + assetNode.usedSymbols, + '*', + nullthrows(outgoingDep.usedSymbolsDown.get(s)), ); - setAddAll(assetNode.usedSymbols, '*', new Set()); + reexportedSymbols.set(s, {asset: assetNode.id, symbol: s}); + } else { + reexportedSymbols.set(s, sResolved); + reexportedSymbolsSource.set(s, outgoingDep); } - setAddAll(reexportedSymbols, s, sDeps); - reexportedSymbolsSource.set(s, outgoingDep); }); + } else { + let targetAsset = outgoingDep.usedSymbolsUp.values().next()?.value; + for (let a of outgoingDep.usedSymbolsUp.values()) { + if (targetAsset != a) targetAsset = null; + } + if (targetAsset != null) { + replacements.set(outgoingDep.id, targetAsset); + } } } } @@ -544,7 +569,7 @@ export class AssetGraphBuilder { if (!incomingDepSymbols) continue; let hasNamespaceReexport = incomingDepSymbols.get('*')?.local === '*'; - for (let [s, sDeps] of incomingDep.usedSymbolsDown) { + for (let [s] of incomingDep.usedSymbolsDown) { if ( assetSymbols == null || // Assume everything could be provided if symbols are cleared assetNode.value.bundleBehavior === BundleBehavior.isolated || @@ -553,7 +578,17 @@ export class AssetGraphBuilder { reexportedSymbols.has(s) || s === '*' ) { - setAddAll(incomingDep.usedSymbolsUp, s, sDeps); + if (reexportedSymbols.has(s)) { + incomingDep.usedSymbolsUp.set( + s, + nullthrows(reexportedSymbols.get(s)), + ); + } else { + incomingDep.usedSymbolsUp.set(s, { + asset: assetNode.id, + symbol: s, + }); + } } else if (!hasNamespaceReexport) { let loc = incomingDep.value.symbols?.get(s)?.loc; let [resolutionNodeId] = this.assetGraph.getNodeIdsConnectedFrom( @@ -591,9 +626,7 @@ export class AssetGraphBuilder { } } - if ( - !equalMapSet(incomingDepUsedSymbolsUpOld, incomingDep.usedSymbolsUp) - ) { + if (!equalMap(incomingDepUsedSymbolsUpOld, incomingDep.usedSymbolsUp)) { changedDeps.add(incomingDep); incomingDep.usedSymbolsUpDirtyUp = true; } @@ -630,6 +663,28 @@ export class AssetGraphBuilder { [...dep.usedSymbolsUp].sort(([a], [b]) => a.localeCompare(b)), ); } + + // Do after the fact to not disrupt traversal + for (let [dep, {asset, symbol}] of replacements) { + let parents = this.assetGraph.getNodeIdsConnectedTo( + this.assetGraph.getNodeIdByContentKey(asset), + ); + let assetGroupId; + if (parents.length === 1) { + [assetGroupId] = parents; + invariant( + this.assetGraph.getNode(assetGroupId)?.type === 'asset_group', + ); + } + let depNodeId = this.assetGraph.getNodeIdByContentKey(dep); + this.assetGraph.replaceNodeIdsConnectedTo(depNodeId, [ + assetGroupId ?? this.assetGraph.getNodeIdByContentKey(asset), + ]); + + let depNode = nullthrows(this.assetGraph.getNode(depNodeId)); + invariant(depNode.type === 'dependency'); + depNode.symbolTarget = symbol; + } } propagateSymbolsDown( @@ -917,6 +972,14 @@ export class AssetGraphBuilder { } } +function equalMap(a: $ReadOnlyMap, b: $ReadOnlyMap) { + return ( + a.size === b.size && + [...a].every(([k, v]) => { + return b.has(k) && b.get(k) === v; + }) + ); +} function equalMapSet( a: $ReadOnlyMap>, b: $ReadOnlyMap>, diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index 1f5868cd226..113e2f759fc 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -308,7 +308,8 @@ export type DependencyNode = {| hasDeferred?: boolean, // symbol -> the dependency that originally requested it usedSymbolsDown: Map>, - usedSymbolsUp: Map>, + // a requested symbol -> the asset it resolved to, and the potentially renamed export name + usedSymbolsUp: Map, /** for the "down" pass, the dependency resolution asset needs to be updated */ usedSymbolsDownDirty: boolean, /** for the "up" pass, the parent asset needs to be updated */ @@ -317,6 +318,9 @@ export type DependencyNode = {| usedSymbolsUpDirtyDown: boolean, /** dependency was excluded (= no used symbols (globally) & side-effect free) */ excluded: boolean, + /** a dependency importing a single symbol is rewritten to point to the reexported target asset + * instead, this is the name of the export (might have been renamed by reexports) */ + symbolTarget: ?Symbol, |}; export type RootNode = {|id: ContentKey, +type: 'root', value: string | null|}; diff --git a/packages/core/types/index.js b/packages/core/types/index.js index 29aa68ee9fd..a7ebdb5953b 100644 --- a/packages/core/types/index.js +++ b/packages/core/types/index.js @@ -1438,6 +1438,7 @@ export interface BundleGraph { symbol: Symbol, boundary: ?Bundle, ): SymbolResolution; + getDependencySymbolTarget(dependency: Dependency): ?Symbol; /** Returns a list of symbols that are exported by the asset, including re-exports. */ getExportedSymbols( asset: Asset, diff --git a/packages/packagers/js/src/ScopeHoistingPackager.js b/packages/packagers/js/src/ScopeHoistingPackager.js index b990058b634..3b3e6e234c0 100644 --- a/packages/packagers/js/src/ScopeHoistingPackager.js +++ b/packages/packagers/js/src/ScopeHoistingPackager.js @@ -758,6 +758,10 @@ ${code} imported: string, dep?: Dependency, ): string { + if (dep != null) { + imported = this.bundleGraph.getDependencySymbolTarget(dep) ?? imported; + } + let { asset: resolvedAsset, exportSymbol, From f1f0a5d8a8878855b1050f9a938ff14a2f733412 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 15:10:36 +0200 Subject: [PATCH 03/48] revert usedSymbolsDown to Set --- packages/core/core/src/AssetGraph.js | 6 +- packages/core/core/src/dumpGraphToGraphViz.js | 16 +--- .../core/src/requests/AssetGraphRequest.js | 81 +++++-------------- packages/core/core/src/types.js | 5 +- 4 files changed, 28 insertions(+), 80 deletions(-) diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index 832162761bd..f4131ac392c 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -55,7 +55,7 @@ export function nodeFromDep(dep: Dependency): DependencyNode { value: dep, deferred: false, excluded: false, - usedSymbolsDown: new Map(), + usedSymbolsDown: new Set(), usedSymbolsUp: new Map(), usedSymbolsDownDirty: true, usedSymbolsUpDirtyDown: true, @@ -88,7 +88,7 @@ export function nodeFromAsset(asset: Asset): AssetNode { id: asset.id, type: 'asset', value: asset, - usedSymbols: new Map(), + usedSymbols: new Set(), usedSymbolsDownDirty: true, usedSymbolsUpDirty: true, }; @@ -257,7 +257,7 @@ export default class AssetGraph extends ContentGraph { if (node.value.env.isLibrary) { // in library mode, all of the entry's symbols are "used" - node.usedSymbolsDown.set('*', new Set()); + node.usedSymbolsDown.add('*'); } return node; }); diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 06e4238edaf..1edf10d384b 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -114,12 +114,7 @@ export default async function dumpGraphToGraphViz( } if (node.usedSymbolsDown.size > 0) { label += - '\\nusedSymbolsDown: ' + - [...node.usedSymbolsDown] - .map(([s, sDeps]) => - sDeps.size > 0 ? `${s}(${[...sDeps].join(',')})` : s, - ) - .join(','); + '\\nusedSymbolsDown: ' + [...node.usedSymbolsDown].join(','); } } else { label += '\\nsymbols: cleared'; @@ -141,14 +136,7 @@ export default async function dumpGraphToGraphViz( .join(';'); } if (node.usedSymbols.size) { - label += - '\\nusedSymbols: ' + - [...node.usedSymbols] - - .map(([s, sDeps]) => - sDeps.size > 0 ? `${s}(${[...sDeps].join(',')})` : s, - ) - .join(','); + label += '\\nusedSymbols: ' + [...node.usedSymbols].join(','); } } else { label += '\\nsymbols: cleared'; diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 870bb34d7d3..d96db432323 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -294,15 +294,15 @@ export class AssetGraphBuilder { let isEntry = false; // Used symbols that are exported or reexported (symbol will be removed again later) by asset. - assetNode.usedSymbols = new Map(); + assetNode.usedSymbols = new Set(); // Symbols that have to be namespace reexported by outgoingDeps. - let namespaceReexportedSymbols = new Map>(); + let namespaceReexportedSymbols = new Set(); if (incomingDeps.length === 0) { // Root in the runtimes Graph - setAddAll(assetNode.usedSymbols, '*', new Set()); - setAddAll(namespaceReexportedSymbols, '*', new Set()); + assetNode.usedSymbols.add('*'); + namespaceReexportedSymbols.add('*'); } else { for (let incomingDep of incomingDeps) { if (incomingDep.value.symbols == null) { @@ -310,13 +310,10 @@ export class AssetGraphBuilder { continue; } - for (let [ - exportSymbol, - exportSymbolDeps, - ] of incomingDep.usedSymbolsDown) { + for (let exportSymbol of incomingDep.usedSymbolsDown) { if (exportSymbol === '*') { - setAddAll(assetNode.usedSymbols, '*', exportSymbolDeps); - setAddAll(namespaceReexportedSymbols, '*', exportSymbolDeps); + assetNode.usedSymbols.add('*'); + namespaceReexportedSymbols.add('*'); } if ( !assetSymbols || @@ -324,18 +321,14 @@ export class AssetGraphBuilder { assetSymbols.has('*') ) { // An own symbol or a non-namespace reexport - setAddAll(assetNode.usedSymbols, exportSymbol, exportSymbolDeps); + assetNode.usedSymbols.add(exportSymbol); } // A namespace reexport // (but only if we actually have namespace-exporting outgoing dependencies, // This usually happens with a reexporting asset with many namespace exports which means that // we cannot match up the correct asset with the used symbol at this level.) else if (hasNamespaceOutgoingDeps && exportSymbol !== 'default') { - setAddAll( - namespaceReexportedSymbols, - exportSymbol, - exportSymbolDeps, - ); + namespaceReexportedSymbols.add(exportSymbol); } } } @@ -345,7 +338,7 @@ export class AssetGraphBuilder { // ---------------------------------------------------------- for (let dep of outgoingDeps) { let depUsedSymbolsDownOld = dep.usedSymbolsDown; - let depUsedSymbolsDown = new Map(); + let depUsedSymbolsDown = new Set(); dep.usedSymbolsDown = depUsedSymbolsDown; if ( assetNode.value.sideEffects || @@ -363,9 +356,9 @@ export class AssetGraphBuilder { if (!depSymbols) continue; if (depSymbols.get('*')?.local === '*') { - for (let [s, sDeps] of namespaceReexportedSymbols) { + for (let s of namespaceReexportedSymbols) { // We need to propagate the namespaceReexportedSymbols to all namespace dependencies (= even wrong ones because we don't know yet) - setAddAll(depUsedSymbolsDown, s, sDeps); + depUsedSymbolsDown.add(s); } } @@ -375,19 +368,15 @@ export class AssetGraphBuilder { if (!assetSymbolsInverse || !depSymbols.get(symbol)?.isWeak) { // Bailout or non-weak symbol (= used in the asset itself = not a reexport) - setAddAll(depUsedSymbolsDown, symbol, new Set([dep.id])); + depUsedSymbolsDown.add(symbol); } else { let reexportedExportSymbols = assetSymbolsInverse.get(local); if (reexportedExportSymbols == null) { // not reexported = used in asset itself - setAddAll(depUsedSymbolsDown, symbol, new Set([dep.id])); + depUsedSymbolsDown.add(symbol); } else if (assetNode.usedSymbols.has('*')) { // we need everything - setAddAll( - depUsedSymbolsDown, - symbol, - nullthrows(assetNode.usedSymbols.get('*')), - ); + depUsedSymbolsDown.add(symbol); [...reexportedExportSymbols].forEach(s => assetNode.usedSymbols.delete(s), @@ -398,7 +387,7 @@ export class AssetGraphBuilder { ].filter(s => assetNode.usedSymbols.has(s)); if (usedReexportedExportSymbols.length > 0) { // The symbol is indeed a reexport, so it's not used from the asset itself - setAddAll(depUsedSymbolsDown, symbol, new Set([dep.id])); + depUsedSymbolsDown.add(symbol); usedReexportedExportSymbols.forEach(s => assetNode.usedSymbols.delete(s), @@ -410,7 +399,7 @@ export class AssetGraphBuilder { } else { depUsedSymbolsDown.clear(); } - if (!equalMapSet(depUsedSymbolsDownOld, depUsedSymbolsDown)) { + if (!equalSet(depUsedSymbolsDownOld, depUsedSymbolsDown)) { dep.usedSymbolsDownDirty = true; dep.usedSymbolsUpDirtyDown = true; } @@ -499,11 +488,7 @@ export class AssetGraphBuilder { outgoingDep, ); } - setAddAll( - assetNode.usedSymbols, - '*', - nullthrows(outgoingDep.usedSymbolsDown.get(s)), - ); + assetNode.usedSymbols.add('*'); reexportedSymbols.set(s, {asset: assetNode.id, symbol: s}); } else { reexportedSymbols.set(s, sResolved); @@ -537,11 +522,7 @@ export class AssetGraphBuilder { outgoingDep, ); } - setAddAll( - assetNode.usedSymbols, - '*', - nullthrows(outgoingDep.usedSymbolsDown.get(s)), - ); + assetNode.usedSymbols.add('*'); reexportedSymbols.set(s, {asset: assetNode.id, symbol: s}); } else { reexportedSymbols.set(s, sResolved); @@ -569,7 +550,7 @@ export class AssetGraphBuilder { if (!incomingDepSymbols) continue; let hasNamespaceReexport = incomingDepSymbols.get('*')?.local === '*'; - for (let [s] of incomingDep.usedSymbolsDown) { + for (let s of incomingDep.usedSymbolsDown) { if ( assetSymbols == null || // Assume everything could be provided if symbols are cleared assetNode.value.bundleBehavior === BundleBehavior.isolated || @@ -980,27 +961,7 @@ function equalMap(a: $ReadOnlyMap, b: $ReadOnlyMap) { }) ); } -function equalMapSet( - a: $ReadOnlyMap>, - b: $ReadOnlyMap>, -) { - return ( - a.size === b.size && - [...a].every(([k, v]) => { - let vB = b.get(k); - return vB && equalSet(v, vB); - }) - ); -} + function equalSet(a: $ReadOnlySet, b: $ReadOnlySet) { return a.size === b.size && [...a].every(i => b.has(i)); } - -function setAddAll(a: Map>, key: K, b: $ReadOnlySet) { - let target = a.get(key); - if (target == null) { - target = new Set(b); - a.set(key, target); - } - b.forEach(v => nullthrows(target).add(v)); -} diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index 113e2f759fc..44855fddaf8 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -290,7 +290,7 @@ export type AssetNode = {| id: ContentKey, +type: 'asset', value: Asset, - usedSymbols: Map>, + usedSymbols: Set, hasDeferred?: boolean, usedSymbolsDownDirty: boolean, usedSymbolsUpDirty: boolean, @@ -306,8 +306,7 @@ export type DependencyNode = {| deferred: boolean, /** dependency was deferred (= no used symbols (in immediate parents) & side-effect free) */ hasDeferred?: boolean, - // symbol -> the dependency that originally requested it - usedSymbolsDown: Map>, + usedSymbolsDown: Set, // a requested symbol -> the asset it resolved to, and the potentially renamed export name usedSymbolsUp: Map, /** for the "down" pass, the dependency resolution asset needs to be updated */ From 7c04c82bad3884a805894d01bbfe0ed2387b4d6c Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 15:51:30 +0200 Subject: [PATCH 04/48] Globally renamed symbols --- packages/core/core/src/AssetGraph.js | 2 +- packages/core/core/src/BundleGraph.js | 20 +++- packages/core/core/src/public/BundleGraph.js | 10 +- packages/core/core/src/public/Symbols.js | 104 +++++++++++++++++- .../core/src/requests/AssetGraphRequest.js | 28 +++-- packages/core/core/src/types.js | 2 +- packages/core/types/index.js | 26 ++++- packages/packagers/css/src/CSSPackager.js | 10 +- .../packagers/js/src/ScopeHoistingPackager.js | 12 +- 9 files changed, 185 insertions(+), 29 deletions(-) diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index f4131ac392c..732e8c1368c 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -60,7 +60,7 @@ export function nodeFromDep(dep: Dependency): DependencyNode { usedSymbolsDownDirty: true, usedSymbolsUpDirtyDown: true, usedSymbolsUpDirtyUp: true, - symbolTarget: null, + symbolTarget: new Map(), }; } diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 67f0fd841b7..0768d019595 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -858,7 +858,9 @@ export default class BundleGraph { }); } - getDependenciesWithSymbolTarget(asset: Asset): Array<[Dependency, ?Symbol]> { + getDependenciesWithSymbolTarget( + asset: Asset, + ): Array<[Dependency, Map]> { let nodeId = this._graph.getNodeIdByContentKey(asset.id); return this._graph.getNodeIdsConnectedFrom(nodeId).map(id => { let node = nullthrows(this._graph.getNode(id)); @@ -1423,7 +1425,10 @@ export default class BundleGraph { let symbolLookup = new Map( [...depSymbols].map(([key, val]) => [val.local, key]), ); - let depSymbol = symbolLookup.get(symbolTarget ?? identifier); + let depSymbol = + identifier != null + ? symbolLookup.get(symbolTarget.get(identifier) ?? identifier) + : undefined; if (depSymbol != null) { let resolved = this.getResolvedAsset(dep); if (!resolved || resolved.id === asset.id) { @@ -1753,9 +1758,14 @@ export default class BundleGraph { getUsedSymbolsDependency(dep: Dependency): ?$ReadOnlySet { let node = this._graph.getNodeByContentKey(dep.id); invariant(node && node.type === 'dependency'); - return this._symbolPropagationRan - ? makeReadOnlySet(new Set(node.usedSymbolsUp.keys())) - : null; + let result = new Set(node.usedSymbolsUp.keys()); + for (let [k, v] of node.symbolTarget) { + if (result.has(k)) { + result.delete(k); + result.add(v); + } + } + return this._symbolPropagationRan ? makeReadOnlySet(result) : null; } merge(other: BundleGraph) { diff --git a/packages/core/core/src/public/BundleGraph.js b/packages/core/core/src/public/BundleGraph.js index ae6938a474c..1d665425c3d 100644 --- a/packages/core/core/src/public/BundleGraph.js +++ b/packages/core/core/src/public/BundleGraph.js @@ -10,6 +10,7 @@ import type { ExportSymbolResolution, FilePath, GraphVisitor, + DependencySymbols as IDependencySymbols, Symbol, SymbolResolution, Target, @@ -27,6 +28,7 @@ import Dependency, {dependencyToInternalDependency} from './Dependency'; import {targetToInternalTarget} from './Target'; import {fromInternalSourceLocation} from '../utils'; import BundleGroup, {bundleGroupToInternalBundleGroup} from './BundleGroup'; +import {DependencySymbols} from './Symbols'; // Friendly access for other modules within this package that need access // to the internal bundle. @@ -236,7 +238,7 @@ export default class BundleGraph }; } - getDependencySymbolTarget(dependency: IDependency): ?Symbol { + getDependencySymbolTarget(dependency: IDependency): Map { let node = nullthrows( this.#graph._graph.getNodeByContentKey(dependency.id), ); @@ -316,6 +318,12 @@ export default class BundleGraph } } + getSymbols(dep: IDependency): IDependencySymbols { + let node = this.#graph._graph.getNodeByContentKey(dep.id); + invariant(node && node.type === 'dependency'); + return new DependencySymbols(this.#options, node.value, node.symbolTarget); + } + getEntryRoot(target: Target): FilePath { return this.#graph.getEntryRoot( this.#options.projectRoot, diff --git a/packages/core/core/src/public/Symbols.js b/packages/core/core/src/public/Symbols.js index 82ae6e3ae70..cf1a041f7e2 100644 --- a/packages/core/core/src/public/Symbols.js +++ b/packages/core/core/src/public/Symbols.js @@ -4,6 +4,7 @@ import type { MutableAssetSymbols as IMutableAssetSymbols, AssetSymbols as IAssetSymbols, MutableDependencySymbols as IMutableDependencySymbols, + DependencySymbols as IDependencySymbols, SourceLocation, Meta, } from '@parcel/types'; @@ -27,7 +28,6 @@ const EMPTY_ITERATOR = { const inspect = Symbol.for('nodejs.util.inspect.custom'); let valueToSymbols: WeakMap = new WeakMap(); - export class AssetSymbols implements IAssetSymbols { /*:: @@iterator(): Iterator<[ISymbol, {|local: ISymbol, loc: ?SourceLocation, meta?: ?Meta|}]> { return ({}: any); } @@ -192,6 +192,108 @@ export class MutableAssetSymbols implements IMutableAssetSymbols { } } +let valueToDependencySymbols: WeakMap = + new WeakMap(); +export class DependencySymbols implements IDependencySymbols { + /*:: + @@iterator(): Iterator<[ISymbol, {|local: ISymbol, loc: ?SourceLocation, isWeak: boolean, meta?: ?Meta|}]> { return ({}: any); } + */ + #value: Dependency; + #options: ParcelOptions; + #symbolTarget: Map; + + constructor( + options: ParcelOptions, + dep: Dependency, + symbolTarget: Map, + ): DependencySymbols { + let existing = valueToDependencySymbols.get(dep); + if (existing != null) { + return existing; + } + this.#value = dep; + this.#options = options; + this.#symbolTarget = symbolTarget; + return this; + } + + #translateExportSymbol(exportSymbol: ISymbol): ISymbol { + return this.#symbolTarget.get(exportSymbol) ?? exportSymbol; + } + + // immutable: + + hasExportSymbol(exportSymbol: ISymbol): boolean { + return Boolean( + this.#value.symbols?.has(this.#translateExportSymbol(exportSymbol)), + ); + } + + hasLocalSymbol(local: ISymbol): boolean { + if (this.#value.symbols) { + for (let s of this.#value.symbols.values()) { + if (local === s.local) return true; + } + } + return false; + } + + get( + exportSymbol: ISymbol, + ): ?{|local: ISymbol, loc: ?SourceLocation, isWeak: boolean, meta?: ?Meta|} { + return fromInternalDependencySymbol( + this.#options.projectRoot, + nullthrows(this.#value.symbols).get( + this.#translateExportSymbol(exportSymbol), + ), + ); + } + + get isCleared(): boolean { + return this.#value.symbols == null; + } + + exportSymbols(): Iterable { + let symbols = this.#value.symbols; + if (symbols) { + let result = new Set(); + for (let s of symbols.keys()) { + result.add(this.#translateExportSymbol(s)); + } + return result; + } else { + // $FlowFixMe + return EMPTY_ITERABLE; + } + } + + // $FlowFixMe + [Symbol.iterator]() { + let symbols = this.#value.symbols; + if (symbols) { + let result = []; + for (let [s, v] of symbols) { + result.push([this.#translateExportSymbol(s), v]); + } + return result[Symbol.iterator](); + } else { + // $FlowFixMe + return EMPTY_ITERABLE; + } + } + + // $FlowFixMe + [inspect]() { + return `DependencySymbols(${ + this.#value.symbols + ? [...this.#value.symbols] + .map(([s, {local, isWeak}]) => `${s}:${local}${isWeak ? '?' : ''}`) + .join(', ') + : null + })`; + } +} + let valueToMutableDependencySymbols: WeakMap< Dependency, MutableDependencySymbols, diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index d96db432323..b75dff1e1c6 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -497,6 +497,8 @@ export class AssetGraphBuilder { }); } + let outgoingDepReplacements = new Map(); + let outgoingDepResolved = undefined; for (let [s, sResolved] of outgoingDep.usedSymbolsUp) { if (!outgoingDep.usedSymbolsDown.has(s)) { // usedSymbolsDown is a superset of usedSymbolsUp @@ -530,15 +532,25 @@ export class AssetGraphBuilder { } }); } else { - let targetAsset = outgoingDep.usedSymbolsUp.values().next()?.value; - for (let a of outgoingDep.usedSymbolsUp.values()) { - if (targetAsset != a) targetAsset = null; - } - if (targetAsset != null) { - replacements.set(outgoingDep.id, targetAsset); + if (outgoingDepResolved === undefined) { + outgoingDepResolved = sResolved.asset; + } else { + if ( + outgoingDepResolved != null && + outgoingDepResolved != sResolved.asset + ) { + outgoingDepResolved = null; + } } + outgoingDepReplacements.set(s, sResolved.symbol ?? s); } } + if (outgoingDepResolved != null) { + replacements.set(outgoingDep.id, { + asset: outgoingDepResolved, + targets: outgoingDepReplacements, + }); + } } let errors: Array = []; @@ -646,7 +658,7 @@ export class AssetGraphBuilder { } // Do after the fact to not disrupt traversal - for (let [dep, {asset, symbol}] of replacements) { + for (let [dep, {asset, targets}] of replacements) { let parents = this.assetGraph.getNodeIdsConnectedTo( this.assetGraph.getNodeIdByContentKey(asset), ); @@ -664,7 +676,7 @@ export class AssetGraphBuilder { let depNode = nullthrows(this.assetGraph.getNode(depNodeId)); invariant(depNode.type === 'dependency'); - depNode.symbolTarget = symbol; + depNode.symbolTarget = targets; } } diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index 44855fddaf8..f7b87e9c50a 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -319,7 +319,7 @@ export type DependencyNode = {| excluded: boolean, /** a dependency importing a single symbol is rewritten to point to the reexported target asset * instead, this is the name of the export (might have been renamed by reexports) */ - symbolTarget: ?Symbol, + symbolTarget: Map, |}; export type RootNode = {|id: ContentKey, +type: 'root', value: string | null|}; diff --git a/packages/core/types/index.js b/packages/core/types/index.js index a7ebdb5953b..bdf34cd0ed8 100644 --- a/packages/core/types/index.js +++ b/packages/core/types/index.js @@ -465,6 +465,29 @@ export interface MutableDependencySymbols // eslint-disable-next-line no-undef delete(exportSymbol: Symbol): void; } +export interface DependencySymbols // eslint-disable-next-line no-undef + extends Iterable< + [ + Symbol, + {|local: Symbol, loc: ?SourceLocation, isWeak: boolean, meta?: ?Meta|}, + ], + > { + /** + * The symbols taht are imports are unknown, rather than just empty. + * This is the default state. + */ + +isCleared: boolean; + get(exportSymbol: Symbol): ?{| + local: Symbol, + loc: ?SourceLocation, + isWeak: boolean, + meta?: ?Meta, + |}; + hasExportSymbol(exportSymbol: Symbol): boolean; + hasLocalSymbol(local: Symbol): boolean; + exportSymbols(): Iterable; +} + export type DependencyPriority = 'sync' | 'parallel' | 'lazy'; export type SpecifierType = 'commonjs' | 'esm' | 'url' | 'custom'; @@ -1438,7 +1461,7 @@ export interface BundleGraph { symbol: Symbol, boundary: ?Bundle, ): SymbolResolution; - getDependencySymbolTarget(dependency: Dependency): ?Symbol; + getDependencySymbolTarget(dependency: Dependency): Map; /** Returns a list of symbols that are exported by the asset, including re-exports. */ getExportedSymbols( asset: Asset, @@ -1450,6 +1473,7 @@ export interface BundleGraph { * Returns null if symbol propagation didn't run (so the result is unknown). */ getUsedSymbols(Asset | Dependency): ?$ReadOnlySet; + getSymbols(Dependency): DependencySymbols; /** Returns the common root directory for the entry assets of a target. */ getEntryRoot(target: Target): FilePath; } diff --git a/packages/packagers/css/src/CSSPackager.js b/packages/packagers/css/src/CSSPackager.js index c602e9044bf..eafc307c3dc 100644 --- a/packages/packagers/css/src/CSSPackager.js +++ b/packages/packagers/css/src/CSSPackager.js @@ -79,7 +79,9 @@ export default (new Packager({ if (asset.meta.hasReferences) { let replacements = new Map(); for (let dep of asset.getDependencies()) { - for (let [exported, {local}] of dep.symbols) { + for (let [exported, {local}] of bundleGraph.getSymbols( + dep, + )) { let resolved = bundleGraph.getResolvedAsset(dep, bundle); if (resolved) { let resolution = bundleGraph.getSymbolResolution( @@ -216,9 +218,11 @@ async function processCSSModule( let defaultImport = null; if (usedSymbols.has('default')) { let incoming = bundleGraph.getIncomingDependencies(asset); - defaultImport = incoming.find(d => d.symbols.hasExportSymbol('default')); + defaultImport = incoming.find(d => + bundleGraph.getSymbols(d).hasExportSymbol('default'), + ); if (defaultImport) { - let loc = defaultImport.symbols.get('default')?.loc; + let loc = bundleGraph.getSymbols(defaultImport).get('default')?.loc; logger.warn({ message: 'CSS modules cannot be tree shaken when imported with a default specifier', diff --git a/packages/packagers/js/src/ScopeHoistingPackager.js b/packages/packagers/js/src/ScopeHoistingPackager.js index 3b3e6e234c0..92f4afa399f 100644 --- a/packages/packagers/js/src/ScopeHoistingPackager.js +++ b/packages/packagers/js/src/ScopeHoistingPackager.js @@ -620,7 +620,7 @@ ${code} continue; } - for (let [imported, {local}] of dep.symbols) { + for (let [imported, {local}] of this.bundleGraph.getSymbols(dep)) { if (local === '*') { continue; } @@ -695,7 +695,7 @@ ${code} this.externals.set(dep.specifier, external); } - for (let [imported, {local}] of dep.symbols) { + for (let [imported, {local}] of this.bundleGraph.getSymbols(dep)) { // If already imported, just add the already renamed variable to the mapping. let renamed = external.get(imported); if (renamed && local !== '*' && replacements) { @@ -758,10 +758,6 @@ ${code} imported: string, dep?: Dependency, ): string { - if (dep != null) { - imported = this.bundleGraph.getDependencySymbolTarget(dep) ?? imported; - } - let { asset: resolvedAsset, exportSymbol, @@ -1045,7 +1041,7 @@ ${code} let isWrapped = resolved && resolved.meta.shouldWrap; - for (let [imported, {local}] of dep.symbols) { + for (let [imported, {local}] of this.bundleGraph.getSymbols(dep)) { if (imported === '*' && local === '*') { if (!resolved) { // Re-exporting an external module. This should have already been handled in buildReplacements. @@ -1206,7 +1202,7 @@ ${code} dep => this.bundle.hasDependency(dep) && // dep.meta.isES6Module && - dep.symbols.hasExportSymbol('default'), + this.bundleGraph.getSymbols(dep).hasExportSymbol('default'), ); } From f389a7f49403d2dcdbf9ced9d71fbe374339b41c Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 15:54:30 +0200 Subject: [PATCH 05/48] Fixup --- packages/core/core/src/BundleGraph.js | 37 +++++++++------------------ 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 0768d019595..6e498a11da7 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -1778,35 +1778,22 @@ export default class BundleGraph { let existingNode = nullthrows(this._graph.getNode(existingNodeId)); // Merge symbols, recompute dep.exluded based on that - // eslint-disable-next-line no-inner-declarations - function merge( - a: Map>, - b: Map>, - ): Map> { - let result = new Map(a); - for (let [k, v] of b) { - let existing = result.get(k); - result.set(k, existing ? new Set([...existing, ...v]) : v); - } - return result; - } - if (existingNode.type === 'asset') { invariant(otherNode.type === 'asset'); - existingNode.usedSymbols = merge( - existingNode.usedSymbols, - otherNode.usedSymbols, - ); + existingNode.usedSymbols = new Set([ + ...existingNode.usedSymbols, + ...otherNode.usedSymbols, + ]); } else if (existingNode.type === 'dependency') { invariant(otherNode.type === 'dependency'); - existingNode.usedSymbolsDown = merge( - existingNode.usedSymbolsDown, - otherNode.usedSymbolsDown, - ); - existingNode.usedSymbolsUp = merge( - existingNode.usedSymbolsUp, - otherNode.usedSymbolsUp, - ); + existingNode.usedSymbolsDown = new Set([ + ...existingNode.usedSymbolsDown, + ...otherNode.usedSymbolsDown, + ]); + existingNode.usedSymbolsUp = new Map([ + ...existingNode.usedSymbolsUp, + ...otherNode.usedSymbolsUp, + ]); existingNode.excluded = (existingNode.excluded || Boolean(existingNode.hasDeferred)) && From 8bb75b5a9ba956406f57ae4e6e256887f31022e7 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 16:08:25 +0200 Subject: [PATCH 06/48] Fixup --- .../core/src/requests/AssetGraphRequest.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index b75dff1e1c6..36a21dc5e8e 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -965,13 +965,17 @@ export class AssetGraphBuilder { } } -function equalMap(a: $ReadOnlyMap, b: $ReadOnlyMap) { - return ( - a.size === b.size && - [...a].every(([k, v]) => { - return b.has(k) && b.get(k) === v; - }) - ); +function equalMap( + a: $ReadOnlyMap, + b: $ReadOnlyMap, +) { + if (a.size !== b.size) return false; + for (let [k, v] of a) { + let vB = b.get(k); + if (vB == null) return false; + if (vB?.asset !== v.asset || vB?.symbol !== v.symbol) return false; + } + return true; } function equalSet(a: $ReadOnlySet, b: $ReadOnlySet) { From f62b1f42ba5e0df4d1b012344c3f3e9a95e83081 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 16:15:59 +0200 Subject: [PATCH 07/48] Stringify dep priority --- packages/core/core/src/dumpGraphToGraphViz.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 1edf10d384b..40df45d3dd4 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -80,8 +80,13 @@ export default async function dumpGraphToGraphViz( if (node.type === 'dependency') { label += node.value.specifier; let parts = []; - if (node.value.priority !== Priority.sync) - parts.push(node.value.priority); + if (node.value.priority !== Priority.sync) { + parts.push( + Object.entries(Priority).find( + ([, v]) => v === node.value.priority, + )?.[0], + ); + } if (node.value.isOptional) parts.push('optional'); if (node.value.specifierType === SpecifierType.url) parts.push('url'); if (node.hasDeferred) parts.push('deferred'); From 3af446d26feab2b6aaf5a9f121a9d87cba61813e Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 16:28:52 +0200 Subject: [PATCH 08/48] process.env.PARCEL_SYMBOLS_CODESPLIT --- packages/core/core/src/requests/AssetGraphRequest.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 36a21dc5e8e..17511ca9f81 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -209,6 +209,10 @@ export class AssetGraphBuilder { d => d.value.env.shouldScopeHoist, ); if (this.assetGraph.symbolPropagationRan) { + dumpToGraphViz( + this.assetGraph, + 'AssetGraph_' + this.name + '_before_prop', + ); try { this.propagateSymbols(); } catch (e) { @@ -659,6 +663,13 @@ export class AssetGraphBuilder { // Do after the fact to not disrupt traversal for (let [dep, {asset, targets}] of replacements) { + if ( + process.env.PARCEL_BUILD_ENV !== 'production' && + // $FlowFixMe + process.env.PARCEL_SYMBOLS_CODESPLIT == false + ) { + break; + } let parents = this.assetGraph.getNodeIdsConnectedTo( this.assetGraph.getNodeIdByContentKey(asset), ); From cdad7c1c1f0014f914750fdfe42511189e783eec Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 16:31:27 +0200 Subject: [PATCH 09/48] Respect side effects when forwarding --- .../core/src/requests/AssetGraphRequest.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 17511ca9f81..18087e55ddd 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -495,7 +495,13 @@ export class AssetGraphBuilder { assetNode.usedSymbols.add('*'); reexportedSymbols.set(s, {asset: assetNode.id, symbol: s}); } else { - reexportedSymbols.set(s, sResolved); + reexportedSymbols.set( + s, + // Forward a reexport only if the current asset is side-effect free. + !assetNode.value.sideEffects + ? sResolved + : {asset: assetNode.id, symbol: s}, + ); reexportedSymbolsSource.set(s, outgoingDep); } }); @@ -531,7 +537,13 @@ export class AssetGraphBuilder { assetNode.usedSymbols.add('*'); reexportedSymbols.set(s, {asset: assetNode.id, symbol: s}); } else { - reexportedSymbols.set(s, sResolved); + reexportedSymbols.set( + s, + // Forward a reexport only if the current asset is side-effect free. + !assetNode.value.sideEffects + ? sResolved + : {asset: assetNode.id, symbol: s}, + ); reexportedSymbolsSource.set(s, outgoingDep); } }); @@ -575,7 +587,8 @@ export class AssetGraphBuilder { reexportedSymbols.has(s) || s === '*' ) { - if (reexportedSymbols.has(s)) { + // Forward a reexport only if the current asset is side-effect free. + if (reexportedSymbols.has(s) && !assetNode.value.sideEffects) { incomingDep.usedSymbolsUp.set( s, nullthrows(reexportedSymbols.get(s)), From 9abdd07fedf636e39e66ba34776e94a4c57dc6fa Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 16:44:18 +0200 Subject: [PATCH 10/48] Externals --- packages/core/core/src/dumpGraphToGraphViz.js | 7 +++--- .../core/src/requests/AssetGraphRequest.js | 22 +++++++++---------- packages/core/core/src/types.js | 4 ++-- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 40df45d3dd4..30188c5eb1b 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -111,9 +111,10 @@ export default async function dumpGraphToGraphViz( label += '\\nusedSymbolsUp: ' + [...node.usedSymbolsUp] - .map( - ([s, sAsset]) => - `${s}(${sAsset.asset}.${sAsset.symbol ?? ''})`, + .map(([s, sAsset]) => + sAsset + ? `${s}(${sAsset.asset}.${sAsset.symbol ?? ''})` + : `${s}(external)`, ) .join(','); } diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 18087e55ddd..01216d8755c 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -458,7 +458,7 @@ export class AssetGraphBuilder { // the symbols that are reexported (not used in `asset`) -> asset they resolved to let reexportedSymbols = new Map< Symbol, - {|asset: ContentKey, symbol: ?Symbol|}, + ?{|asset: ContentKey, symbol: ?Symbol|}, >(); // the symbols that are reexported (not used in `asset`) -> the corresponding outgoingDep(s) // To generate the diagnostic when there are multiple dependencies with non-statically @@ -468,14 +468,14 @@ export class AssetGraphBuilder { let outgoingDepSymbols = outgoingDep.value.symbols; if (!outgoingDepSymbols) continue; - // excluded, assume everything that is requested exists - if ( + let isExcluded = this.assetGraph.getNodeIdsConnectedFrom( this.assetGraph.getNodeIdByContentKey(outgoingDep.id), - ).length === 0 - ) { + ).length === 0; + // excluded, assume everything that is requested exists + if (isExcluded) { outgoingDep.usedSymbolsDown.forEach((_, s) => - outgoingDep.usedSymbolsUp.set(s, {asset: assetNode.id, symbol: s}), + outgoingDep.usedSymbolsUp.set(s, null), ); } @@ -547,7 +547,7 @@ export class AssetGraphBuilder { reexportedSymbolsSource.set(s, outgoingDep); } }); - } else { + } else if (sResolved) { if (outgoingDepResolved === undefined) { outgoingDepResolved = sResolved.asset; } else { @@ -990,14 +990,14 @@ export class AssetGraphBuilder { } function equalMap( - a: $ReadOnlyMap, - b: $ReadOnlyMap, + a: $ReadOnlyMap, + b: $ReadOnlyMap, ) { if (a.size !== b.size) return false; for (let [k, v] of a) { + if (!b.has(k)) return false; let vB = b.get(k); - if (vB == null) return false; - if (vB?.asset !== v.asset || vB?.symbol !== v.symbol) return false; + if (vB?.asset !== v?.asset || vB?.symbol !== v?.symbol) return false; } return true; } diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index f7b87e9c50a..883b8e35330 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -307,8 +307,8 @@ export type DependencyNode = {| /** dependency was deferred (= no used symbols (in immediate parents) & side-effect free) */ hasDeferred?: boolean, usedSymbolsDown: Set, - // a requested symbol -> the asset it resolved to, and the potentially renamed export name - usedSymbolsUp: Map, + // a requested symbol -> the asset it resolved to, and the potentially renamed export name (null if external) + usedSymbolsUp: Map, /** for the "down" pass, the dependency resolution asset needs to be updated */ usedSymbolsDownDirty: boolean, /** for the "up" pass, the parent asset needs to be updated */ From 0bbc8de28bec932586623217106d21fe888de44c Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 16:49:41 +0200 Subject: [PATCH 11/48] Use current asset for ns import --- packages/core/core/src/requests/AssetGraphRequest.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 01216d8755c..c5aaf448566 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -583,12 +583,16 @@ export class AssetGraphBuilder { assetSymbols == null || // Assume everything could be provided if symbols are cleared assetNode.value.bundleBehavior === BundleBehavior.isolated || assetNode.value.bundleBehavior === BundleBehavior.inline || - assetNode.usedSymbols.has(s) || - reexportedSymbols.has(s) || - s === '*' + s === '*' || + assetNode.usedSymbols.has(s) ) { + incomingDep.usedSymbolsUp.set(s, { + asset: assetNode.id, + symbol: s, + }); + } else if (reexportedSymbols.has(s)) { // Forward a reexport only if the current asset is side-effect free. - if (reexportedSymbols.has(s) && !assetNode.value.sideEffects) { + if (!assetNode.value.sideEffects) { incomingDep.usedSymbolsUp.set( s, nullthrows(reexportedSymbols.get(s)), From a198221c53cb56715050a7eab179e696bb81f7aa Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 16:56:57 +0200 Subject: [PATCH 12/48] Remove unnecessarily strict assertions --- .../integration-tests/test/scope-hoisting.js | 33 ++----------------- 1 file changed, 2 insertions(+), 31 deletions(-) diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js index ab0927df6f6..dd918b9242d 100644 --- a/packages/core/integration-tests/test/scope-hoisting.js +++ b/packages/core/integration-tests/test/scope-hoisting.js @@ -6,7 +6,6 @@ import {createWorkerFarm} from '@parcel/core'; import {md} from '@parcel/diagnostic'; import { assertBundles, - assertDependencyWasExcluded, bundle as _bundle, bundler as _bundler, distDir, @@ -2441,12 +2440,6 @@ describe('scope hoisting', function () { let output = await run(bundleEvent.bundleGraph); assert.deepEqual(output, [123]); - assertDependencyWasExcluded( - bundleEvent.bundleGraph, - 'a.js', - './c.js', - ); - await overlayFS.copyFile( path.join(testDir, 'index.2.js'), path.join(testDir, 'index.js'), @@ -2674,18 +2667,6 @@ describe('scope hoisting', function () { ), new Set(['gridSize']), ); - assert.deepStrictEqual( - new Set( - bundleEvent.bundleGraph.getUsedSymbols( - findDependency( - bundleEvent.bundleGraph, - 'theme.js', - './themeColors', - ), - ), - ), - new Set(), - ); assert(!findAsset(bundleEvent.bundleGraph, 'themeColors.js')); await overlayFS.copyFile( @@ -2725,6 +2706,7 @@ describe('scope hoisting', function () { ), new Set('*'), ); + assert(findAsset(bundleEvent.bundleGraph, 'themeColors.js')); await overlayFS.copyFile( path.join(testDir, 'index.1.js'), @@ -2748,18 +2730,7 @@ describe('scope hoisting', function () { ), new Set(['gridSize']), ); - assert.deepStrictEqual( - new Set( - bundleEvent.bundleGraph.getUsedSymbols( - findDependency( - bundleEvent.bundleGraph, - 'theme.js', - './themeColors', - ), - ), - ), - new Set(), - ); + assert(!findAsset(bundleEvent.bundleGraph, 'themeColors.js')); } finally { await subscription.unsubscribe(); } From cacb9cfac6a3892b52cd82a31fa1c7c99db13220 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 11 Aug 2022 17:19:34 +0200 Subject: [PATCH 13/48] Edge types for asset graph --- packages/core/core/src/AssetGraph.js | 15 +++++++++++++-- packages/core/core/src/dumpGraphToGraphViz.js | 10 ++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index 732e8c1368c..a4a913fee15 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -30,6 +30,14 @@ import {ContentGraph} from '@parcel/graph'; import {createDependency} from './Dependency'; import {type ProjectPath, fromProjectPathRelative} from './projectPath'; +export const assetGraphEdgeTypes = { + null: 1, + //TODO + original: 2, +}; + +export type AssetGraphEdgeType = $Values; + type InitOpts = {| entries?: Array, targets?: Array, @@ -43,7 +51,7 @@ type AssetGraphOpts = {| |}; type SerializedAssetGraph = {| - ...SerializedContentGraph, + ...SerializedContentGraph, hash?: ?string, symbolPropagationRan: boolean, |}; @@ -110,7 +118,10 @@ export function nodeFromEntryFile(entry: Entry): EntryFileNode { }; } -export default class AssetGraph extends ContentGraph { +export default class AssetGraph extends ContentGraph< + AssetGraphNode, + AssetGraphEdgeType, +> { onNodeRemoved: ?(nodeId: NodeId) => mixed; hash: ?string; envCache: Map; diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 30188c5eb1b..8e1d03179d3 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -3,6 +3,8 @@ import type {Asset, BundleBehavior} from '@parcel/types'; import type {Graph} from '@parcel/graph'; import type {AssetGraphNode, BundleGraphNode, Environment} from './types'; +import type {AssetGraphEdgeType} from './AssetGraph'; +import {assetGraphEdgeTypes} from './AssetGraph'; import {bundleGraphEdgeTypes} from './BundleGraph'; import {requestGraphEdgeTypes} from './RequestTracker'; @@ -26,6 +28,7 @@ const TYPE_COLORS = { internal_async: 'orange', references: 'red', sibling: 'green', + original: 'grey', invalidated_by_create: 'green', invalidated_by_create_above: 'orange', invalidate_by_update: 'cyan', @@ -34,7 +37,7 @@ const TYPE_COLORS = { export default async function dumpGraphToGraphViz( graph: - | Graph + | Graph | Graph<{| assets: Set, sourceBundles: Set, @@ -42,7 +45,10 @@ export default async function dumpGraphToGraphViz( |}> | Graph, name: string, - edgeTypes?: typeof bundleGraphEdgeTypes | typeof requestGraphEdgeTypes, + edgeTypes?: + | typeof assetGraphEdgeTypes + | typeof bundleGraphEdgeTypes + | typeof requestGraphEdgeTypes, ): Promise { if ( process.env.PARCEL_BUILD_ENV === 'production' || From 83b223e7057a5fe76fa7921a57ff97a108aa6750 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 12 Aug 2022 14:18:06 +0200 Subject: [PATCH 14/48] Remove getDependencySymbolTarget --- packages/core/core/src/public/BundleGraph.js | 8 -------- packages/core/types/index.js | 1 - 2 files changed, 9 deletions(-) diff --git a/packages/core/core/src/public/BundleGraph.js b/packages/core/core/src/public/BundleGraph.js index 1d665425c3d..3f1e07aeaf0 100644 --- a/packages/core/core/src/public/BundleGraph.js +++ b/packages/core/core/src/public/BundleGraph.js @@ -238,14 +238,6 @@ export default class BundleGraph }; } - getDependencySymbolTarget(dependency: IDependency): Map { - let node = nullthrows( - this.#graph._graph.getNodeByContentKey(dependency.id), - ); - invariant(node.type === 'dependency'); - return node.symbolTarget; - } - getExportedSymbols( asset: IAsset, boundary: ?IBundle, diff --git a/packages/core/types/index.js b/packages/core/types/index.js index bdf34cd0ed8..feb993bd59e 100644 --- a/packages/core/types/index.js +++ b/packages/core/types/index.js @@ -1461,7 +1461,6 @@ export interface BundleGraph { symbol: Symbol, boundary: ?Bundle, ): SymbolResolution; - getDependencySymbolTarget(dependency: Dependency): Map; /** Returns a list of symbols that are exported by the asset, including re-exports. */ getExportedSymbols( asset: Asset, From ca4febe6c1a5d6c36970a23fcea11e4ce6db6f00 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 12 Aug 2022 14:18:50 +0200 Subject: [PATCH 15/48] Nullable symbolTarget --- packages/core/core/src/AssetGraph.js | 2 +- packages/core/core/src/BundleGraph.js | 14 ++++++++------ packages/core/core/src/public/Symbols.js | 6 +++--- packages/core/core/src/types.js | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index a4a913fee15..bb157752a54 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -68,7 +68,7 @@ export function nodeFromDep(dep: Dependency): DependencyNode { usedSymbolsDownDirty: true, usedSymbolsUpDirtyDown: true, usedSymbolsUpDirtyUp: true, - symbolTarget: new Map(), + symbolTarget: null, }; } diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 6e498a11da7..e92d02d1b2a 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -860,7 +860,7 @@ export default class BundleGraph { getDependenciesWithSymbolTarget( asset: Asset, - ): Array<[Dependency, Map]> { + ): Array<[Dependency, ?Map]> { let nodeId = this._graph.getNodeIdByContentKey(asset.id); return this._graph.getNodeIdsConnectedFrom(nodeId).map(id => { let node = nullthrows(this._graph.getNode(id)); @@ -1427,7 +1427,7 @@ export default class BundleGraph { ); let depSymbol = identifier != null - ? symbolLookup.get(symbolTarget.get(identifier) ?? identifier) + ? symbolLookup.get(symbolTarget?.get(identifier) ?? identifier) : undefined; if (depSymbol != null) { let resolved = this.getResolvedAsset(dep); @@ -1759,10 +1759,12 @@ export default class BundleGraph { let node = this._graph.getNodeByContentKey(dep.id); invariant(node && node.type === 'dependency'); let result = new Set(node.usedSymbolsUp.keys()); - for (let [k, v] of node.symbolTarget) { - if (result.has(k)) { - result.delete(k); - result.add(v); + if (node.symbolTarget) { + for (let [k, v] of node.symbolTarget) { + if (result.has(k)) { + result.delete(k); + result.add(v); + } } } return this._symbolPropagationRan ? makeReadOnlySet(result) : null; diff --git a/packages/core/core/src/public/Symbols.js b/packages/core/core/src/public/Symbols.js index cf1a041f7e2..26c52f1b6af 100644 --- a/packages/core/core/src/public/Symbols.js +++ b/packages/core/core/src/public/Symbols.js @@ -200,12 +200,12 @@ export class DependencySymbols implements IDependencySymbols { */ #value: Dependency; #options: ParcelOptions; - #symbolTarget: Map; + #symbolTarget: ?Map; constructor( options: ParcelOptions, dep: Dependency, - symbolTarget: Map, + symbolTarget: ?Map, ): DependencySymbols { let existing = valueToDependencySymbols.get(dep); if (existing != null) { @@ -218,7 +218,7 @@ export class DependencySymbols implements IDependencySymbols { } #translateExportSymbol(exportSymbol: ISymbol): ISymbol { - return this.#symbolTarget.get(exportSymbol) ?? exportSymbol; + return this.#symbolTarget?.get(exportSymbol) ?? exportSymbol; } // immutable: diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index 883b8e35330..e826f50f52a 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -319,7 +319,7 @@ export type DependencyNode = {| excluded: boolean, /** a dependency importing a single symbol is rewritten to point to the reexported target asset * instead, this is the name of the export (might have been renamed by reexports) */ - symbolTarget: Map, + symbolTarget: ?Map, |}; export type RootNode = {|id: ContentKey, +type: 'root', value: string | null|}; From 523d04e4fd2afe1b1077888c7ab96141bb0e8642 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 12 Aug 2022 14:23:37 +0200 Subject: [PATCH 16/48] Also remove redirected edges again --- .../core/src/requests/AssetGraphRequest.js | 73 +++++++++++++------ 1 file changed, 51 insertions(+), 22 deletions(-) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index c5aaf448566..de3d4d4e8b1 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -27,7 +27,7 @@ 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'; +import AssetGraph, {assetGraphEdgeTypes} from '../AssetGraph'; import {PARCEL_VERSION} from '../constants'; import createEntryRequest from './EntryRequest'; import createTargetRequest from './TargetRequest'; @@ -38,8 +38,7 @@ import { fromProjectPathRelative, fromProjectPath, } from '../projectPath'; - -import dumpToGraphViz from '../dumpGraphToGraphViz'; +import dumpGraphToGraphViz from '../dumpGraphToGraphViz'; type AssetGraphRequestInput = {| entries?: Array, @@ -209,18 +208,27 @@ export class AssetGraphBuilder { d => d.value.env.shouldScopeHoist, ); if (this.assetGraph.symbolPropagationRan) { - dumpToGraphViz( + await dumpGraphToGraphViz( this.assetGraph, 'AssetGraph_' + this.name + '_before_prop', + assetGraphEdgeTypes, ); try { this.propagateSymbols(); } catch (e) { - dumpToGraphViz(this.assetGraph, 'AssetGraph_' + this.name + '_failed'); + await dumpGraphToGraphViz( + this.assetGraph, + 'AssetGraph_' + this.name + '_failed', + assetGraphEdgeTypes, + ); throw e; } } - dumpToGraphViz(this.assetGraph, 'AssetGraph_' + this.name); + await dumpGraphToGraphViz( + this.assetGraph, + 'AssetGraph_' + this.name, + assetGraphEdgeTypes, + ); return { assetGraph: this.assetGraph, @@ -566,6 +574,9 @@ export class AssetGraphBuilder { asset: outgoingDepResolved, targets: outgoingDepReplacements, }); + } else { + // Edge needs to be removed again + replacements.set(outgoingDep.id, null); } } @@ -679,7 +690,7 @@ export class AssetGraphBuilder { } // Do after the fact to not disrupt traversal - for (let [dep, {asset, targets}] of replacements) { + for (let [dep, replacement] of replacements) { if ( process.env.PARCEL_BUILD_ENV !== 'production' && // $FlowFixMe @@ -687,24 +698,42 @@ export class AssetGraphBuilder { ) { break; } - let parents = this.assetGraph.getNodeIdsConnectedTo( - this.assetGraph.getNodeIdByContentKey(asset), - ); - let assetGroupId; - if (parents.length === 1) { - [assetGroupId] = parents; - invariant( - this.assetGraph.getNode(assetGroupId)?.type === 'asset_group', - ); - } - let depNodeId = this.assetGraph.getNodeIdByContentKey(dep); - this.assetGraph.replaceNodeIdsConnectedTo(depNodeId, [ - assetGroupId ?? this.assetGraph.getNodeIdByContentKey(asset), - ]); + let depNodeId = this.assetGraph.getNodeIdByContentKey(dep); let depNode = nullthrows(this.assetGraph.getNode(depNodeId)); invariant(depNode.type === 'dependency'); - depNode.symbolTarget = targets; + if (replacement) { + let {asset, targets} = replacement; + let assetParents = this.assetGraph.getNodeIdsConnectedTo( + this.assetGraph.getNodeIdByContentKey(asset), + ); + let assetGroupId; + if (assetParents.length === 1) { + [assetGroupId] = assetParents; + invariant( + this.assetGraph.getNode(assetGroupId)?.type === 'asset_group', + ); + } + + this.assetGraph.addEdge( + depNodeId, + assetGroupId ?? this.assetGraph.getNodeIdByContentKey(asset), + assetGraphEdgeTypes.redirected, + ); + depNode.symbolTarget = targets; + } else { + for (let n of this.assetGraph.getNodeIdsConnectedFrom( + depNodeId, + assetGraphEdgeTypes.redirected, + )) { + this.assetGraph.removeEdge( + depNodeId, + n, + assetGraphEdgeTypes.redirected, + ); + } + depNode.symbolTarget = null; + } } } From 97dc341fc7f398acc98f4afaa93dfd1e0b7796ff Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 12 Aug 2022 14:24:46 +0200 Subject: [PATCH 17/48] Prefer redirected edges for bundle graph --- packages/core/core/src/AssetGraph.js | 5 +- packages/core/core/src/BundleGraph.js | 121 ++++++++++-------- packages/core/core/src/dumpGraphToGraphViz.js | 5 +- 3 files changed, 77 insertions(+), 54 deletions(-) diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index bb157752a54..72563b13b05 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -32,8 +32,9 @@ import {type ProjectPath, fromProjectPathRelative} from './projectPath'; export const assetGraphEdgeTypes = { null: 1, - //TODO - original: 2, + // In addition to the null edge, a dependency can be connected to the asset containing the symbols + // that the dependency requested (after reexports were skipped). + redirected: 2, }; export type AssetGraphEdgeType = $Values; diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index e92d02d1b2a..93f5354a44d 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -37,6 +37,7 @@ import {Priority, BundleBehavior, SpecifierType} from './types'; import {getBundleGroupId, getPublicId} from './utils'; import {ISOLATED_ENVS} from './public/Environment'; import {fromProjectPath} from './projectPath'; +import {assetGraphEdgeTypes} from './AssetGraph'; export const bundleGraphEdgeTypes = { // A lack of an edge type indicates to follow the edge while traversing @@ -146,7 +147,8 @@ export default class BundleGraph { assetPublicIds: Set = new Set(), ): BundleGraph { let graph = new ContentGraph(); - let assetGroupIds = new Set(); + let assetGroupIds = new Map(); + let dependencyIds = new Set(); let assetGraphNodeIdToBundleGraphNodeId = new Map(); let assetGraphRootNode = @@ -155,63 +157,80 @@ export default class BundleGraph { : null; invariant(assetGraphRootNode != null && assetGraphRootNode.type === 'root'); - for (let [nodeId, node] of assetGraph.nodes) { - if (node.type === 'asset') { - let {id: assetId} = node.value; - // Generate a new, short public id for this asset to use. - // If one already exists, use it. - let publicId = publicIdByAssetId.get(assetId); - if (publicId == null) { - publicId = getPublicId(assetId, existing => - assetPublicIds.has(existing), - ); - publicIdByAssetId.set(assetId, publicId); - assetPublicIds.add(publicId); + assetGraph.dfs({ + visit: nodeId => { + let node = nullthrows(assetGraph.getNode(nodeId)); + if (node.type === 'asset') { + let {id: assetId} = node.value; + // Generate a new, short public id for this asset to use. + // If one already exists, use it. + let publicId = publicIdByAssetId.get(assetId); + if (publicId == null) { + publicId = getPublicId(assetId, existing => + assetPublicIds.has(existing), + ); + publicIdByAssetId.set(assetId, publicId); + assetPublicIds.add(publicId); + } } - } - // Don't copy over asset groups into the bundle graph. - if (node.type === 'asset_group') { - assetGroupIds.add(nodeId); - } else { - let bundleGraphNodeId = graph.addNodeByContentKey(node.id, node); - if (node.id === assetGraphRootNode?.id) { - graph.setRootNodeId(bundleGraphNodeId); + // Don't copy over asset groups into the bundle graph. + if (node.type === 'asset_group') { + assetGroupIds.set( + nodeId, + assetGraph.getNodeIdsConnectedFrom( + nodeId, + assetGraphEdgeTypes.null, + ), + ); + } else { + if (node.type === 'dependency') { + dependencyIds.add(nodeId); + } + let bundleGraphNodeId = graph.addNodeByContentKey(node.id, node); + if (node.id === assetGraphRootNode?.id) { + graph.setRootNodeId(bundleGraphNodeId); + } + assetGraphNodeIdToBundleGraphNodeId.set(nodeId, bundleGraphNodeId); } - assetGraphNodeIdToBundleGraphNodeId.set(nodeId, bundleGraphNodeId); - } - } + }, + startNodeId: null, + getChildren: nodeId => { + let children = assetGraph.getNodeIdsConnectedFrom( + nodeId, + assetGraphEdgeTypes.redirected, + ); + if (children.length > 0) { + return children; + } else { + return assetGraph.getNodeIdsConnectedFrom( + nodeId, + assetGraphEdgeTypes.null, + ); + } + }, + }); for (let edge of assetGraph.getAllEdges()) { - let fromIds; - if (assetGroupIds.has(edge.from)) { - fromIds = [ - ...assetGraph.getNodeIdsConnectedTo( - edge.from, - bundleGraphEdgeTypes.null, - ), - ]; - } else { - fromIds = [edge.from]; + if ( + dependencyIds.has(edge.from) && + edge.type === assetGraphEdgeTypes.null && + assetGraph.adjacencyList + .getOutboundEdgesByType(edge.from) + .some(n => n.type === assetGraphEdgeTypes.redirected) + ) { + // If there's a redirect edge for a dependency, ignore the normal edge and convert the + // redirect edge into a regular bundlegraph edge. + continue; } - for (let from of fromIds) { - if (assetGroupIds.has(edge.to)) { - for (let to of assetGraph.getNodeIdsConnectedFrom( - edge.to, - bundleGraphEdgeTypes.null, - )) { - graph.addEdge( - nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(from)), - nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(to)), - ); - } - } else { - graph.addEdge( - nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(from)), - nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(edge.to)), - ); - } + let fromBundleGraph = assetGraphNodeIdToBundleGraphNodeId.get(edge.from); + if (fromBundleGraph == null) continue; // an asset group + for (let to of assetGroupIds.get(edge.to) ?? [edge.to]) { + graph.addEdge( + fromBundleGraph, + nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(to)), + ); } } diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 8e1d03179d3..105d66174f1 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -23,12 +23,15 @@ const COLORS = { }; const TYPE_COLORS = { + // bundle graph bundle: 'blue', contains: 'grey', internal_async: 'orange', references: 'red', sibling: 'green', - original: 'grey', + // asset graph + redirected: 'grey', + // request graph invalidated_by_create: 'green', invalidated_by_create_above: 'orange', invalidate_by_update: 'cyan', From 01e5cef7338a1c0f75678eddb8f642921f7959c7 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 12 Aug 2022 15:36:29 +0200 Subject: [PATCH 18/48] Dump symbolTarget --- packages/core/core/src/dumpGraphToGraphViz.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 105d66174f1..b32d602925c 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -131,6 +131,11 @@ export default async function dumpGraphToGraphViz( label += '\\nusedSymbolsDown: ' + [...node.usedSymbolsDown].join(','); } + if (node.symbolTarget && node.symbolTarget?.size > 0) { + label += + '\\nsymbolTarget: ' + + [...node.symbolTarget].map(([a, b]) => `${a}:${b}`).join(','); + } } else { label += '\\nsymbols: cleared'; } From 63efc43111adee3cc4cad0ea403795d0560dc5e0 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 12 Aug 2022 16:11:54 +0200 Subject: [PATCH 19/48] Add test --- .../index.js | 3 ++ .../library/a.js | 2 + .../library/b.js | 2 + .../library/dynamic.js | 4 ++ .../library/index.js | 4 ++ .../library/package.json | 3 ++ .../core/integration-tests/test/javascript.js | 37 +++++++++++++++++++ 7 files changed, 55 insertions(+) create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/index.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/a.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/b.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/dynamic.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/index.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/package.json diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/index.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/index.js new file mode 100644 index 00000000000..3888e43d6b2 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/index.js @@ -0,0 +1,3 @@ +import { Context } from "./library"; + +output = [Context, () => import("./library/dynamic")]; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/a.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/a.js new file mode 100644 index 00000000000..9eda04b81ad --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/a.js @@ -0,0 +1,2 @@ +sideEffect("a"); +export const Ctx = 1; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/b.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/b.js new file mode 100644 index 00000000000..18b74695dea --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/b.js @@ -0,0 +1,2 @@ +sideEffect("b"); +export const Ctx = 2; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/dynamic.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/dynamic.js new file mode 100644 index 00000000000..74c81826df6 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/dynamic.js @@ -0,0 +1,4 @@ +sideEffect("dynamic"); +import { Ctx } from "./a.js"; +import { id } from "./index.js"; +export default [Ctx, id]; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/index.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/index.js new file mode 100644 index 00000000000..21b2f50e4ab --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/index.js @@ -0,0 +1,4 @@ +sideEffect("index"); +export { Ctx } from "./a.js"; +export { Ctx as Context } from "./b.js"; +export const id = 3; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/package.json b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/package.json new file mode 100644 index 00000000000..1b95642997c --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/package.json @@ -0,0 +1,3 @@ +{ + "sideEffects": false +} diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js index 8cedde288af..18aa7fc5b57 100644 --- a/packages/core/integration-tests/test/javascript.js +++ b/packages/core/integration-tests/test/javascript.js @@ -7186,6 +7186,43 @@ describe('javascript', function () { assert.deepEqual(res.output, ['foo', 'bar']); }); + it('supports partially used reexporting index file', async function () { + let b = await bundle( + path.join( + __dirname, + '/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/index.js', + ), + options, + ); + + let calls = []; + let res = ( + await run( + b, + { + sideEffect: caller => { + calls.push(caller); + }, + }, + {require: false}, + ) + ).output; + + let [v, async] = res; + + assert.deepEqual(calls, shouldScopeHoist ? ['b'] : ['a', 'b', 'index']); + assert.deepEqual(v, 2); + + v = await async(); + assert.deepEqual( + calls, + shouldScopeHoist + ? ['b', 'a', 'index', 'dynamic'] + : ['a', 'b', 'index', 'dynamic'], + ); + assert.deepEqual(v.default, [1, 3]); + }); + it('supports deferring non-weak dependencies that are not used', async function () { let b = await bundle( path.join( From 30d1361a23b0d6cfaecbbecb574c8f949534aa82 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Wed, 17 Aug 2022 10:17:57 +0200 Subject: [PATCH 20/48] Fixup assertions --- .../core/integration-tests/test/javascript.js | 66 +++++++++++-------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js index 18aa7fc5b57..348cc87216b 100644 --- a/packages/core/integration-tests/test/javascript.js +++ b/packages/core/integration-tests/test/javascript.js @@ -6246,7 +6246,7 @@ describe('javascript', function () { }, { type: 'js', - assets: ['index.js', 'a.js', 'b1.js'], + assets: ['index.js', 'b1.js'], }, { type: 'css', @@ -6291,7 +6291,7 @@ describe('javascript', function () { }, { type: 'js', - assets: ['index.js', 'a.js', 'b1.js'], + assets: ['index.js', 'b1.js'], }, ]); @@ -6320,7 +6320,21 @@ describe('javascript', function () { options, ); - assertDependencyWasExcluded(b, 'index.js', './message2.js'); + assertBundles(b, [ + { + type: 'js', + assets: usesSymbolPropagation + ? ['a.js', 'message1.js'] + : [ + 'a.js', + 'esmodule-helpers.js', + 'index.js', + 'message1.js', + 'message3.js', + ], + }, + ]); + if (usesSymbolPropagation) { // TODO this only excluded, but should be deferred. assert(!findAsset(b, 'message3.js')); @@ -6403,10 +6417,24 @@ describe('javascript', function () { options, ); + assertBundles(b, [ + { + type: 'js', + assets: usesSymbolPropagation + ? ['c.js', 'message3.js'] + : [ + 'c.js', + 'esmodule-helpers.js', + 'index.js', + 'message1.js', + 'message3.js', + ], + }, + ]); + if (usesSymbolPropagation) { assert(!findAsset(b, 'message1.js')); } - assertDependencyWasExcluded(b, 'index.js', './message2.js'); let calls = []; let res = await run( @@ -6707,11 +6735,9 @@ describe('javascript', function () { ); assert(!called, 'side effect called'); assert.deepEqual(res.output, 4); - assertDependencyWasExcluded( - bundleEvent.bundleGraph, - 'index.js', - './bar', - ); + if (usesSymbolPropagation) { + assert(!findAsset(bundleEvent.bundleGraph, 'index.js')); + } await overlayFS.mkdirp(path.join(testDir, 'node_modules/bar')); await overlayFS.copyFile( @@ -7082,10 +7108,7 @@ describe('javascript', function () { ); if (usesSymbolPropagation) { - assert.deepStrictEqual( - new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'index.js')))), - new Set([]), - ); + assert(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'other.js')))), new Set(['default']), @@ -7120,10 +7143,7 @@ describe('javascript', function () { ); if (usesSymbolPropagation) { - assert.deepStrictEqual( - new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'index.js')))), - new Set([]), - ); + assert(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'other.js')))), new Set(['bar']), @@ -7158,10 +7178,7 @@ describe('javascript', function () { ); if (usesSymbolPropagation) { - assert.deepStrictEqual( - new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'index.js')))), - new Set([]), - ); + assert(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'other.js')))), new Set(['default', 'bar']), @@ -7306,17 +7323,12 @@ describe('javascript', function () { if (usesSymbolPropagation) { assert(!findAsset(b, 'esm.js')); + assert(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'commonjs.js')))), // the exports object is used freely new Set(['*', 'message2']), ); - assert.deepEqual( - new Set( - b.getUsedSymbols(findDependency(b, 'index.js', './commonjs.js')), - ), - new Set(['message2']), - ); } let calls = []; From 89e32650cc0be7b8e111d05c578314821601a883 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Wed, 17 Aug 2022 10:27:01 +0200 Subject: [PATCH 21/48] Disable for async imports for now --- packages/core/core/src/requests/AssetGraphRequest.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index de3d4d4e8b1..445c9649bad 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -569,7 +569,15 @@ export class AssetGraphBuilder { outgoingDepReplacements.set(s, sResolved.symbol ?? s); } } - if (outgoingDepResolved != null) { + if ( + // TODO we currently can't replace async imports from + // (parcelRequire("4pwI8")).then(({ a: a })=>a); + // to + // (parcelRequire("4pwI8")).then((a)=>a); + // if symbolTarget == { a -> * } + outgoingDep.value.priority == Priority.sync && + outgoingDepResolved != null + ) { replacements.set(outgoingDep.id, { asset: outgoingDepResolved, targets: outgoingDepReplacements, From 298bdb96118ef9f74a86dfdd56b77ef522aad258 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Wed, 17 Aug 2022 10:33:29 +0200 Subject: [PATCH 22/48] Fixup --- packages/core/core/src/public/Symbols.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/core/src/public/Symbols.js b/packages/core/core/src/public/Symbols.js index 26c52f1b6af..75130c0d885 100644 --- a/packages/core/core/src/public/Symbols.js +++ b/packages/core/core/src/public/Symbols.js @@ -278,7 +278,7 @@ export class DependencySymbols implements IDependencySymbols { return result[Symbol.iterator](); } else { // $FlowFixMe - return EMPTY_ITERABLE; + return EMPTY_ITERATOR; } } From acd949ff91c9e12d9f8e03842106a2f425dd4b14 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Wed, 17 Aug 2022 10:38:45 +0200 Subject: [PATCH 23/48] Fixup --- packages/core/core/src/requests/AssetGraphRequest.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 445c9649bad..839351efae3 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -718,9 +718,13 @@ export class AssetGraphBuilder { let assetGroupId; if (assetParents.length === 1) { [assetGroupId] = assetParents; - invariant( - this.assetGraph.getNode(assetGroupId)?.type === 'asset_group', - ); + let type = this.assetGraph.getNode(assetGroupId)?.type; + // Parent is either an asset group, or a dependency when transformer returned multiple + // connected assets. + if (type !== 'asset_group') { + invariant(type === 'dependency'); + assetGroupId = undefined; + } } this.assetGraph.addEdge( From 3a7fdf98f22a99abf4fba15c33cefb1fdb2dd951 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Wed, 17 Aug 2022 10:42:27 +0200 Subject: [PATCH 24/48] Fixup assertions --- .../test/integration/resolver-canDefer/index.js | 4 ++-- .../test/integration/resolver-canDefer/library/index.js | 1 + packages/core/integration-tests/test/plugin.js | 5 +---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/core/integration-tests/test/integration/resolver-canDefer/index.js b/packages/core/integration-tests/test/integration/resolver-canDefer/index.js index f8f5ca007e9..381a65c3b7c 100644 --- a/packages/core/integration-tests/test/integration/resolver-canDefer/index.js +++ b/packages/core/integration-tests/test/integration/resolver-canDefer/index.js @@ -1,3 +1,3 @@ -import {a} from "./library"; +import {a, index} from "./library"; -output = a; +output = [a, index]; diff --git a/packages/core/integration-tests/test/integration/resolver-canDefer/library/index.js b/packages/core/integration-tests/test/integration/resolver-canDefer/library/index.js index cd00ec2d483..86922c23686 100644 --- a/packages/core/integration-tests/test/integration/resolver-canDefer/library/index.js +++ b/packages/core/integration-tests/test/integration/resolver-canDefer/library/index.js @@ -2,3 +2,4 @@ sideEffect("index"); export {a} from "./a.js"; export {b} from "./b.js"; export {c} from "./c.js"; +export const index = "Index"; diff --git a/packages/core/integration-tests/test/plugin.js b/packages/core/integration-tests/test/plugin.js index 5b86ca5fbe9..7472e28e8f8 100644 --- a/packages/core/integration-tests/test/plugin.js +++ b/packages/core/integration-tests/test/plugin.js @@ -123,10 +123,7 @@ parcel-transformer-b`, }, ); - assert.deepStrictEqual( - new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'index.js')))), - new Set([]), - ); + assert.deepStrictEqual(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'a.js')))), new Set(['a']), From 9ee9ecf7cac4f70b7c97dc225045bb51ccf841c8 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Wed, 17 Aug 2022 10:43:42 +0200 Subject: [PATCH 25/48] Fixup --- packages/core/core/src/requests/AssetGraphRequest.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 839351efae3..f8db5a5d2c6 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -612,10 +612,7 @@ export class AssetGraphBuilder { } else if (reexportedSymbols.has(s)) { // Forward a reexport only if the current asset is side-effect free. if (!assetNode.value.sideEffects) { - incomingDep.usedSymbolsUp.set( - s, - nullthrows(reexportedSymbols.get(s)), - ); + incomingDep.usedSymbolsUp.set(s, reexportedSymbols.get(s)); } else { incomingDep.usedSymbolsUp.set(s, { asset: assetNode.id, From 0d00ab84e27d6dae59fa8e395ea2173bc3346ce2 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 25 Aug 2022 09:44:27 +0200 Subject: [PATCH 26/48] WIP --- packages/core/core/src/BundleGraph.js | 1 - .../core/src/requests/AssetGraphRequest.js | 96 +++++++++------ .../packagers/js/src/ScopeHoistingPackager.js | 111 ++++++++++-------- 3 files changed, 117 insertions(+), 91 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 93f5354a44d..4eacd88898f 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -213,7 +213,6 @@ export default class BundleGraph { for (let edge of assetGraph.getAllEdges()) { if ( - dependencyIds.has(edge.from) && edge.type === assetGraphEdgeTypes.null && assetGraph.adjacencyList .getOutboundEdgesByType(edge.from) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index f8db5a5d2c6..272ac3ab6c6 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -22,7 +22,7 @@ import type {PathRequestInput} from './PathRequest'; import invariant from 'assert'; import nullthrows from 'nullthrows'; -import {PromiseQueue} from '@parcel/utils'; +import {DefaultMap, PromiseQueue} from '@parcel/utils'; import {hashString} from '@parcel/hash'; import logger from '@parcel/logger'; import ThrowableDiagnostic, {md} from '@parcel/diagnostic'; @@ -515,8 +515,10 @@ export class AssetGraphBuilder { }); } - let outgoingDepReplacements = new Map(); - let outgoingDepResolved = undefined; + let outgoingDepReplacements = new DefaultMap< + ContentKey, + Map, + >(() => new Map()); for (let [s, sResolved] of outgoingDep.usedSymbolsUp) { if (!outgoingDep.usedSymbolsDown.has(s)) { // usedSymbolsDown is a superset of usedSymbolsUp @@ -556,17 +558,9 @@ export class AssetGraphBuilder { } }); } else if (sResolved) { - if (outgoingDepResolved === undefined) { - outgoingDepResolved = sResolved.asset; - } else { - if ( - outgoingDepResolved != null && - outgoingDepResolved != sResolved.asset - ) { - outgoingDepResolved = null; - } - } - outgoingDepReplacements.set(s, sResolved.symbol ?? s); + outgoingDepReplacements + .get(sResolved.asset) + .set(s, sResolved.symbol ?? s); } } if ( @@ -576,12 +570,9 @@ export class AssetGraphBuilder { // (parcelRequire("4pwI8")).then((a)=>a); // if symbolTarget == { a -> * } outgoingDep.value.priority == Priority.sync && - outgoingDepResolved != null + outgoingDepReplacements.size > 0 ) { - replacements.set(outgoingDep.id, { - asset: outgoingDepResolved, - targets: outgoingDepReplacements, - }); + replacements.set(outgoingDep.id, outgoingDepReplacements); } else { // Edge needs to be removed again replacements.set(outgoingDep.id, null); @@ -708,29 +699,58 @@ export class AssetGraphBuilder { let depNode = nullthrows(this.assetGraph.getNode(depNodeId)); invariant(depNode.type === 'dependency'); if (replacement) { - let {asset, targets} = replacement; - let assetParents = this.assetGraph.getNodeIdsConnectedTo( - this.assetGraph.getNodeIdByContentKey(asset), + let sourceAssetId = this.assetGraph.getNodeIdByContentKey( + nullthrows(depNode.value.sourceAssetId), ); - let assetGroupId; - if (assetParents.length === 1) { - [assetGroupId] = assetParents; - let type = this.assetGraph.getNode(assetGroupId)?.type; - // Parent is either an asset group, or a dependency when transformer returned multiple - // connected assets. - if (type !== 'asset_group') { - invariant(type === 'dependency'); - assetGroupId = undefined; + for (let [asset, targets] of replacement) { + let assetParents = this.assetGraph.getNodeIdsConnectedTo( + this.assetGraph.getNodeIdByContentKey(asset), + ); + let assetGroupId; + if (assetParents.length === 1) { + [assetGroupId] = assetParents; + let type = this.assetGraph.getNode(assetGroupId)?.type; + // Parent is either an asset group, or a dependency when transformer returned multiple + // connected assets. + if (type !== 'asset_group') { + invariant(type === 'dependency'); + assetGroupId = undefined; + } } - } - this.assetGraph.addEdge( - depNodeId, - assetGroupId ?? this.assetGraph.getNodeIdByContentKey(asset), - assetGraphEdgeTypes.redirected, - ); - depNode.symbolTarget = targets; + let depNodeTarget = + assetGroupId ?? this.assetGraph.getNodeIdByContentKey(asset); + + let newDepId = hashString(depNode.id + [...targets.keys()].join(',')); + let newDep = this.assetGraph.addNode({ + ...depNode, + id: newDepId, + value: { + ...depNode.value, + id: newDepId, + symbols: depNode.value.symbols + ? new Map( + [...depNode.value.symbols].filter(([k]) => targets.has(k)), + ) + : undefined, + }, + usedSymbolsUp: new Map( + [...depNode.usedSymbolsUp].filter(([k]) => targets.has(k)), + ), + usedSymbolsDown: new Set(), + symbolTarget: targets, + }); + + // TODO adjust sourceAssetIdNode.value.dependencies ? + this.assetGraph.addEdge( + sourceAssetId, + newDep, + assetGraphEdgeTypes.redirected, + ); + this.assetGraph.addEdge(newDep, depNodeTarget); + } } else { + // Remove for (let n of this.assetGraph.getNodeIdsConnectedFrom( depNodeId, assetGraphEdgeTypes.redirected, diff --git a/packages/packagers/js/src/ScopeHoistingPackager.js b/packages/packagers/js/src/ScopeHoistingPackager.js index 92f4afa399f..8153dc5a112 100644 --- a/packages/packagers/js/src/ScopeHoistingPackager.js +++ b/packages/packagers/js/src/ScopeHoistingPackager.js @@ -9,6 +9,7 @@ import type { } from '@parcel/types'; import { + DefaultMap, PromiseQueue, relativeBundlePath, countLines, @@ -472,59 +473,64 @@ export class ScopeHoistingPackager { // If we matched an import, replace with the source code for the dependency. if (d != null) { - let dep = depMap.get(d); - if (!dep) { + let deps = depMap.get(d); + if (!deps) { return m; } - let resolved = this.bundleGraph.getResolvedAsset(dep, this.bundle); - let skipped = this.bundleGraph.isDependencySkipped(dep); - if (resolved && !skipped) { - // Hoist variable declarations for the referenced parcelRequire dependencies - // after the dependency is declared. This handles the case where the resulting asset - // is wrapped, but the dependency in this asset is not marked as wrapped. This means - // that it was imported/required at the top-level, so its side effects should run immediately. - let [res, lines] = this.getHoistedParcelRequires( - asset, - dep, - resolved, - ); - let map; - if ( - this.bundle.hasAsset(resolved) && - !this.seenAssets.has(resolved.id) - ) { - // If this asset is wrapped, we need to hoist the code for the dependency - // outside our parcelRequire.register wrapper. This is safe because all - // assets referenced by this asset will also be wrapped. Otherwise, inline the - // asset content where the import statement was. - if (shouldWrap) { - depContent.push(this.visitAsset(resolved)); - } else { - let [depCode, depMap, depLines] = this.visitAsset(resolved); - res = depCode + '\n' + res; - lines += 1 + depLines; - map = depMap; + let replacement = ''; + + // A single `${id}:${specifier}:esm` might have been resolved to multiple assets due to + // reexports. + for (let dep of deps) { + let resolved = this.bundleGraph.getResolvedAsset(dep, this.bundle); + let skipped = this.bundleGraph.isDependencySkipped(dep); + if (resolved && !skipped) { + // Hoist variable declarations for the referenced parcelRequire dependencies + // after the dependency is declared. This handles the case where the resulting asset + // is wrapped, but the dependency in this asset is not marked as wrapped. This means + // that it was imported/required at the top-level, so its side effects should run immediately. + let [res, lines] = this.getHoistedParcelRequires( + asset, + dep, + resolved, + ); + let map; + if ( + this.bundle.hasAsset(resolved) && + !this.seenAssets.has(resolved.id) + ) { + // If this asset is wrapped, we need to hoist the code for the dependency + // outside our parcelRequire.register wrapper. This is safe because all + // assets referenced by this asset will also be wrapped. Otherwise, inline the + // asset content where the import statement was. + if (shouldWrap) { + depContent.push(this.visitAsset(resolved)); + } else { + let [depCode, depMap, depLines] = this.visitAsset(resolved); + res = depCode + '\n' + res; + lines += 1 + depLines; + map = depMap; + } } - } - // Push this asset's source mappings down by the number of lines in the dependency - // plus the number of hoisted parcelRequires. Then insert the source map for the dependency. - if (sourceMap) { - if (lines > 0) { - sourceMap.offsetLines(lineCount + 1, lines); - } + // Push this asset's source mappings down by the number of lines in the dependency + // plus the number of hoisted parcelRequires. Then insert the source map for the dependency. + if (sourceMap) { + if (lines > 0) { + sourceMap.offsetLines(lineCount + 1, lines); + } - if (map) { - sourceMap.addSourceMap(map, lineCount); + if (map) { + sourceMap.addSourceMap(map, lineCount); + } } - } - lineCount += lines; - return res; + replacement += res; + lineCount += lines; + } } - - return ''; + return replacement; } // If it wasn't a dependency, then it was an inline replacement (e.g. $id$import$foo -> $id$export$foo). @@ -580,23 +586,24 @@ ${code} buildReplacements( asset: Asset, deps: Array, - ): [Map, Map] { + ): [Map>, Map] { let assetId = asset.meta.id; invariant(typeof assetId === 'string'); // Build two maps: one of import specifiers, and one of imported symbols to replace. // These will be used to build a regex below. - let depMap = new Map(); + let depMap = new DefaultMap>(() => []); let replacements = new Map(); for (let dep of deps) { let specifierType = dep.specifierType === 'esm' ? `:${dep.specifierType}` : ''; - depMap.set( - `${assetId}:${getSpecifier(dep)}${ - !dep.meta.placeholder ? specifierType : '' - }`, - dep, - ); + depMap + .get( + `${assetId}:${getSpecifier(dep)}${ + !dep.meta.placeholder ? specifierType : '' + }`, + ) + .push(dep); let asyncResolution = this.bundleGraph.resolveAsyncDependency( dep, From 13a2cc183e1e2abb226e0ab48f0bb13e249e79f7 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 25 Aug 2022 16:24:56 +0200 Subject: [PATCH 27/48] f --- packages/core/core/src/BundleGraph.js | 165 +++++++++++++----- .../core/src/requests/AssetGraphRequest.js | 158 ++++++++--------- 2 files changed, 199 insertions(+), 124 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 4eacd88898f..ac35e05127b 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -7,6 +7,7 @@ import type { TraversalActions, } from '@parcel/types'; import type { + ContentKey, ContentGraphOpts, NodeId, SerializedContentGraph, @@ -31,7 +32,7 @@ import invariant from 'assert'; import nullthrows from 'nullthrows'; import {ContentGraph, ALL_EDGE_TYPES, mapVisitor} from '@parcel/graph'; import {Hash, hashString} from '@parcel/hash'; -import {objectSortedEntriesDeep, getRootDir} from '@parcel/utils'; +import {DefaultMap, objectSortedEntriesDeep, getRootDir} from '@parcel/utils'; import {Priority, BundleBehavior, SpecifierType} from './types'; import {getBundleGroupId, getPublicId} from './utils'; @@ -148,7 +149,7 @@ export default class BundleGraph { ): BundleGraph { let graph = new ContentGraph(); let assetGroupIds = new Map(); - let dependencyIds = new Set(); + let dependencies = new Map(); let assetGraphNodeIdToBundleGraphNodeId = new Map(); let assetGraphRootNode = @@ -157,36 +158,93 @@ export default class BundleGraph { : null; invariant(assetGraphRootNode != null && assetGraphRootNode.type === 'root'); + for (let [nodeId, node] of assetGraph.nodes) { + if (node.type === 'asset') { + let {id: assetId} = node.value; + // Generate a new, short public id for this asset to use. + // If one already exists, use it. + let publicId = publicIdByAssetId.get(assetId); + if (publicId == null) { + publicId = getPublicId(assetId, existing => + assetPublicIds.has(existing), + ); + publicIdByAssetId.set(assetId, publicId); + assetPublicIds.add(publicId); + } + } else if (node.type === 'asset_group') { + assetGroupIds.set( + nodeId, + assetGraph.getNodeIdsConnectedFrom(nodeId, assetGraphEdgeTypes.null), + ); + } + } + assetGraph.dfs({ visit: nodeId => { let node = nullthrows(assetGraph.getNode(nodeId)); - if (node.type === 'asset') { - let {id: assetId} = node.value; - // Generate a new, short public id for this asset to use. - // If one already exists, use it. - let publicId = publicIdByAssetId.get(assetId); - if (publicId == null) { - publicId = getPublicId(assetId, existing => - assetPublicIds.has(existing), - ); - publicIdByAssetId.set(assetId, publicId); - assetPublicIds.add(publicId); - } - } - - // Don't copy over asset groups into the bundle graph. if (node.type === 'asset_group') { - assetGroupIds.set( - nodeId, - assetGraph.getNodeIdsConnectedFrom( - nodeId, - assetGraphEdgeTypes.null, - ), + // Don't copy over asset groups into the bundle graph. + return; + } else if (node.type === 'dependency' && node.value.symbols != null) { + // asset -> symbols that should be imported directly from that asset + let targets = new DefaultMap>( + () => new Map(), ); - } else { - if (node.type === 'dependency') { - dependencyIds.add(nodeId); + for (let [symbol, resolvedSymbol] of node.usedSymbolsUp) { + if (resolvedSymbol) { + targets + .get(resolvedSymbol.asset) + .set(symbol, resolvedSymbol.symbol ?? symbol); + } } + // TODO adjust sourceAssetIdNode.value.dependencies ? + if (targets.size > 0) { + dependencies.set( + nodeId, + [...targets].map(([asset, target]) => { + let newNodeId = hashString( + node.id + [...target.keys()].join(','), + ); + return { + asset, + dep: graph.addNodeByContentKey(newNodeId, { + ...node, + id: newNodeId, + value: { + ...node.value, + id: newNodeId, + symbols: node.value.symbols + ? new Map( + [...node.value.symbols].filter( + ([k]) => target.has(k) || k === '*', + ), + ) + : undefined, + }, + usedSymbolsUp: new Map( + [...node.usedSymbolsUp].filter( + ([k]) => target.has(k) || k === '*', + ), + ), + usedSymbolsDown: new Set(), + symbolTarget: target, + }), + }; + }), + ); + } else { + dependencies.set(nodeId, [ + { + asset: null, + dep: graph.addNodeByContentKey(node.id, { + ...node, + symbolTarget: null, + excluded: true, + }), + }, + ]); + } + } else { let bundleGraphNodeId = graph.addNodeByContentKey(node.id, node); if (node.id === assetGraphRootNode?.id) { graph.setRootNodeId(bundleGraphNodeId); @@ -196,12 +254,12 @@ export default class BundleGraph { }, startNodeId: null, getChildren: nodeId => { - let children = assetGraph.getNodeIdsConnectedFrom( - nodeId, - assetGraphEdgeTypes.redirected, - ); - if (children.length > 0) { - return children; + let node = nullthrows(assetGraph.getNode(nodeId)); + if (node.type === 'dependency' && node.value.symbols != null) { + // Jump to the dependencies that are used in this dependencies + return [...node.usedSymbolsUp.values()] + .filter(Boolean) + .map(v => assetGraph.getNodeIdByContentKey(v.asset)); } else { return assetGraph.getNodeIdsConnectedFrom( nodeId, @@ -212,23 +270,40 @@ export default class BundleGraph { }); for (let edge of assetGraph.getAllEdges()) { - if ( - edge.type === assetGraphEdgeTypes.null && - assetGraph.adjacencyList - .getOutboundEdgesByType(edge.from) - .some(n => n.type === assetGraphEdgeTypes.redirected) - ) { - // If there's a redirect edge for a dependency, ignore the normal edge and convert the - // redirect edge into a regular bundlegraph edge. + if (assetGroupIds.has(edge.from)) { continue; } + if (dependencies.has(edge.from)) { + // Discard previous edge, insert outgoing edges for all split dependencies + for (let {asset, dep} of nullthrows(dependencies.get(edge.from))) { + if (asset != null) { + graph.addEdge( + dep, + nullthrows( + assetGraphNodeIdToBundleGraphNodeId.get( + assetGraph.getNodeIdByContentKey(asset), + ), + ), + ); + } + } + continue; + } + if (!assetGraphNodeIdToBundleGraphNodeId.has(edge.from)) { + continue; + } + + let to: Array = dependencies.get(edge.to)?.map(v => v.dep) ?? + assetGroupIds + .get(edge.to) + ?.map(id => + nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(id)), + ) ?? [nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(edge.to))]; - let fromBundleGraph = assetGraphNodeIdToBundleGraphNodeId.get(edge.from); - if (fromBundleGraph == null) continue; // an asset group - for (let to of assetGroupIds.get(edge.to) ?? [edge.to]) { + for (let t of to) { graph.addEdge( - fromBundleGraph, - nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(to)), + nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(edge.from)), + t, ); } } diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 272ac3ab6c6..888f9844d64 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -685,85 +685,85 @@ export class AssetGraphBuilder { ); } - // Do after the fact to not disrupt traversal - for (let [dep, replacement] of replacements) { - if ( - process.env.PARCEL_BUILD_ENV !== 'production' && - // $FlowFixMe - process.env.PARCEL_SYMBOLS_CODESPLIT == false - ) { - break; - } - - let depNodeId = this.assetGraph.getNodeIdByContentKey(dep); - let depNode = nullthrows(this.assetGraph.getNode(depNodeId)); - invariant(depNode.type === 'dependency'); - if (replacement) { - let sourceAssetId = this.assetGraph.getNodeIdByContentKey( - nullthrows(depNode.value.sourceAssetId), - ); - for (let [asset, targets] of replacement) { - let assetParents = this.assetGraph.getNodeIdsConnectedTo( - this.assetGraph.getNodeIdByContentKey(asset), - ); - let assetGroupId; - if (assetParents.length === 1) { - [assetGroupId] = assetParents; - let type = this.assetGraph.getNode(assetGroupId)?.type; - // Parent is either an asset group, or a dependency when transformer returned multiple - // connected assets. - if (type !== 'asset_group') { - invariant(type === 'dependency'); - assetGroupId = undefined; - } - } - - let depNodeTarget = - assetGroupId ?? this.assetGraph.getNodeIdByContentKey(asset); - - let newDepId = hashString(depNode.id + [...targets.keys()].join(',')); - let newDep = this.assetGraph.addNode({ - ...depNode, - id: newDepId, - value: { - ...depNode.value, - id: newDepId, - symbols: depNode.value.symbols - ? new Map( - [...depNode.value.symbols].filter(([k]) => targets.has(k)), - ) - : undefined, - }, - usedSymbolsUp: new Map( - [...depNode.usedSymbolsUp].filter(([k]) => targets.has(k)), - ), - usedSymbolsDown: new Set(), - symbolTarget: targets, - }); - - // TODO adjust sourceAssetIdNode.value.dependencies ? - this.assetGraph.addEdge( - sourceAssetId, - newDep, - assetGraphEdgeTypes.redirected, - ); - this.assetGraph.addEdge(newDep, depNodeTarget); - } - } else { - // Remove - for (let n of this.assetGraph.getNodeIdsConnectedFrom( - depNodeId, - assetGraphEdgeTypes.redirected, - )) { - this.assetGraph.removeEdge( - depNodeId, - n, - assetGraphEdgeTypes.redirected, - ); - } - depNode.symbolTarget = null; - } - } + // // Do after the fact to not disrupt traversal + // for (let [dep, replacement] of replacements) { + // if ( + // process.env.PARCEL_BUILD_ENV !== 'production' && + // // $FlowFixMe + // process.env.PARCEL_SYMBOLS_CODESPLIT == false + // ) { + // break; + // } + + // let depNodeId = this.assetGraph.getNodeIdByContentKey(dep); + // let depNode = nullthrows(this.assetGraph.getNode(depNodeId)); + // invariant(depNode.type === 'dependency'); + // if (replacement) { + // let sourceAssetId = this.assetGraph.getNodeIdByContentKey( + // nullthrows(depNode.value.sourceAssetId), + // ); + // for (let [asset, targets] of replacement) { + // let assetParents = this.assetGraph.getNodeIdsConnectedTo( + // this.assetGraph.getNodeIdByContentKey(asset), + // ); + // let assetGroupId; + // if (assetParents.length === 1) { + // [assetGroupId] = assetParents; + // let type = this.assetGraph.getNode(assetGroupId)?.type; + // // Parent is either an asset group, or a dependency when transformer returned multiple + // // connected assets. + // if (type !== 'asset_group') { + // invariant(type === 'dependency'); + // assetGroupId = undefined; + // } + // } + + // let depNodeTarget = + // assetGroupId ?? this.assetGraph.getNodeIdByContentKey(asset); + + // let newDepId = hashString(depNode.id + [...targets.keys()].join(',')); + // let newDep = this.assetGraph.addNode({ + // ...depNode, + // id: newDepId, + // value: { + // ...depNode.value, + // id: newDepId, + // symbols: depNode.value.symbols + // ? new Map( + // [...depNode.value.symbols].filter(([k]) => targets.has(k)), + // ) + // : undefined, + // }, + // usedSymbolsUp: new Map( + // [...depNode.usedSymbolsUp].filter(([k]) => targets.has(k)), + // ), + // usedSymbolsDown: new Set(), + // symbolTarget: targets, + // }); + + // // TODO adjust sourceAssetIdNode.value.dependencies ? + // this.assetGraph.addEdge( + // sourceAssetId, + // newDep, + // assetGraphEdgeTypes.redirected, + // ); + // this.assetGraph.addEdge(newDep, depNodeTarget); + // } + // } else { + // // Remove + // for (let n of this.assetGraph.getNodeIdsConnectedFrom( + // depNodeId, + // assetGraphEdgeTypes.redirected, + // )) { + // this.assetGraph.removeEdge( + // depNodeId, + // n, + // assetGraphEdgeTypes.redirected, + // ); + // } + // depNode.symbolTarget = null; + // } + // } } propagateSymbolsDown( From eb67f69bc86b4251420d2069020353b98c8e8857 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 25 Aug 2022 16:38:48 +0200 Subject: [PATCH 28/48] f --- packages/core/core/src/BundleGraph.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index ac35e05127b..5eedc318b00 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -190,11 +190,14 @@ export default class BundleGraph { let targets = new DefaultMap>( () => new Map(), ); + let hasExternal = false; for (let [symbol, resolvedSymbol] of node.usedSymbolsUp) { if (resolvedSymbol) { targets .get(resolvedSymbol.asset) .set(symbol, resolvedSymbol.symbol ?? symbol); + } else { + hasExternal = true; } } // TODO adjust sourceAssetIdNode.value.dependencies ? @@ -238,8 +241,8 @@ export default class BundleGraph { asset: null, dep: graph.addNodeByContentKey(node.id, { ...node, - symbolTarget: null, - excluded: true, + // TODO ?? + excluded: !hasExternal, }), }, ]); From d013aa39a174198984aa8378f738a8126e0373cf Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 29 Aug 2022 10:06:20 +0200 Subject: [PATCH 29/48] f --- packages/core/core/src/BundleGraph.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 5eedc318b00..c05413778a9 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -202,9 +202,16 @@ export default class BundleGraph { } // TODO adjust sourceAssetIdNode.value.dependencies ? if (targets.size > 0) { - dependencies.set( - nodeId, - [...targets].map(([asset, target]) => { + let deps = [ + // Keep the original dependency + { + asset: null, + dep: graph.addNodeByContentKey(node.id, { + ...node, + excluded: true, + }), + }, + ...[...targets].map(([asset, target]) => { let newNodeId = hashString( node.id + [...target.keys()].join(','), ); @@ -234,7 +241,8 @@ export default class BundleGraph { }), }; }), - ); + ]; + dependencies.set(nodeId, deps); } else { dependencies.set(nodeId, [ { From 18fc33f7307de6c823dcc84b6b7e109987f781d9 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 29 Aug 2022 10:46:15 +0200 Subject: [PATCH 30/48] Fixup for externals --- packages/core/core/src/requests/AssetGraphRequest.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 888f9844d64..45ad07aa21f 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -601,9 +601,10 @@ export class AssetGraphBuilder { symbol: s, }); } else if (reexportedSymbols.has(s)) { - // Forward a reexport only if the current asset is side-effect free. - if (!assetNode.value.sideEffects) { - incomingDep.usedSymbolsUp.set(s, reexportedSymbols.get(s)); + // Forward a reexport only if the current asset is side-effect free and not external + let reexport = reexportedSymbols.get(s); + if (!assetNode.value.sideEffects && reexport != null) { + incomingDep.usedSymbolsUp.set(s, reexport); } else { incomingDep.usedSymbolsUp.set(s, { asset: assetNode.id, From b2a407da1d5cac5e7d18ee0537fa92ed41702f41 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 29 Aug 2022 10:47:15 +0200 Subject: [PATCH 31/48] Remove replacement part of propagation --- .../core/src/requests/AssetGraphRequest.js | 106 +----------------- 1 file changed, 1 insertion(+), 105 deletions(-) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 45ad07aa21f..5fd3c9bc29c 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -22,7 +22,7 @@ import type {PathRequestInput} from './PathRequest'; import invariant from 'assert'; import nullthrows from 'nullthrows'; -import {DefaultMap, PromiseQueue} from '@parcel/utils'; +import {PromiseQueue} from '@parcel/utils'; import {hashString} from '@parcel/hash'; import logger from '@parcel/logger'; import ThrowableDiagnostic, {md} from '@parcel/diagnostic'; @@ -438,8 +438,6 @@ export class AssetGraphBuilder { } }; - let replacements = new Map(); - // 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) => { @@ -515,10 +513,6 @@ export class AssetGraphBuilder { }); } - let outgoingDepReplacements = new DefaultMap< - ContentKey, - Map, - >(() => new Map()); for (let [s, sResolved] of outgoingDep.usedSymbolsUp) { if (!outgoingDep.usedSymbolsDown.has(s)) { // usedSymbolsDown is a superset of usedSymbolsUp @@ -557,26 +551,8 @@ export class AssetGraphBuilder { reexportedSymbolsSource.set(s, outgoingDep); } }); - } else if (sResolved) { - outgoingDepReplacements - .get(sResolved.asset) - .set(s, sResolved.symbol ?? s); } } - if ( - // TODO we currently can't replace async imports from - // (parcelRequire("4pwI8")).then(({ a: a })=>a); - // to - // (parcelRequire("4pwI8")).then((a)=>a); - // if symbolTarget == { a -> * } - outgoingDep.value.priority == Priority.sync && - outgoingDepReplacements.size > 0 - ) { - replacements.set(outgoingDep.id, outgoingDepReplacements); - } else { - // Edge needs to be removed again - replacements.set(outgoingDep.id, null); - } } let errors: Array = []; @@ -685,86 +661,6 @@ export class AssetGraphBuilder { [...dep.usedSymbolsUp].sort(([a], [b]) => a.localeCompare(b)), ); } - - // // Do after the fact to not disrupt traversal - // for (let [dep, replacement] of replacements) { - // if ( - // process.env.PARCEL_BUILD_ENV !== 'production' && - // // $FlowFixMe - // process.env.PARCEL_SYMBOLS_CODESPLIT == false - // ) { - // break; - // } - - // let depNodeId = this.assetGraph.getNodeIdByContentKey(dep); - // let depNode = nullthrows(this.assetGraph.getNode(depNodeId)); - // invariant(depNode.type === 'dependency'); - // if (replacement) { - // let sourceAssetId = this.assetGraph.getNodeIdByContentKey( - // nullthrows(depNode.value.sourceAssetId), - // ); - // for (let [asset, targets] of replacement) { - // let assetParents = this.assetGraph.getNodeIdsConnectedTo( - // this.assetGraph.getNodeIdByContentKey(asset), - // ); - // let assetGroupId; - // if (assetParents.length === 1) { - // [assetGroupId] = assetParents; - // let type = this.assetGraph.getNode(assetGroupId)?.type; - // // Parent is either an asset group, or a dependency when transformer returned multiple - // // connected assets. - // if (type !== 'asset_group') { - // invariant(type === 'dependency'); - // assetGroupId = undefined; - // } - // } - - // let depNodeTarget = - // assetGroupId ?? this.assetGraph.getNodeIdByContentKey(asset); - - // let newDepId = hashString(depNode.id + [...targets.keys()].join(',')); - // let newDep = this.assetGraph.addNode({ - // ...depNode, - // id: newDepId, - // value: { - // ...depNode.value, - // id: newDepId, - // symbols: depNode.value.symbols - // ? new Map( - // [...depNode.value.symbols].filter(([k]) => targets.has(k)), - // ) - // : undefined, - // }, - // usedSymbolsUp: new Map( - // [...depNode.usedSymbolsUp].filter(([k]) => targets.has(k)), - // ), - // usedSymbolsDown: new Set(), - // symbolTarget: targets, - // }); - - // // TODO adjust sourceAssetIdNode.value.dependencies ? - // this.assetGraph.addEdge( - // sourceAssetId, - // newDep, - // assetGraphEdgeTypes.redirected, - // ); - // this.assetGraph.addEdge(newDep, depNodeTarget); - // } - // } else { - // // Remove - // for (let n of this.assetGraph.getNodeIdsConnectedFrom( - // depNodeId, - // assetGraphEdgeTypes.redirected, - // )) { - // this.assetGraph.removeEdge( - // depNodeId, - // n, - // assetGraphEdgeTypes.redirected, - // ); - // } - // depNode.symbolTarget = null; - // } - // } } propagateSymbolsDown( From 5aaccc1d326866c96b756347fcd41231562a522b Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 29 Aug 2022 10:46:30 +0200 Subject: [PATCH 32/48] f --- packages/core/core/src/BundleGraph.js | 114 ++++++++++++++------------ 1 file changed, 62 insertions(+), 52 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index c05413778a9..dc5215cf6b7 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -190,71 +190,81 @@ export default class BundleGraph { let targets = new DefaultMap>( () => new Map(), ); - let hasExternal = false; + let externalSymbols = new Set(); for (let [symbol, resolvedSymbol] of node.usedSymbolsUp) { if (resolvedSymbol) { targets .get(resolvedSymbol.asset) .set(symbol, resolvedSymbol.symbol ?? symbol); } else { - hasExternal = true; + externalSymbols.add(symbol); } } + + // if (targets.size === 1 && targets.keys().next().value ==) { + // // No special handling + // let bundleGraphNodeId = graph.addNodeByContentKey(node.id, node); + // assetGraphNodeIdToBundleGraphNodeId.set(nodeId, bundleGraphNodeId); + // } else { // TODO adjust sourceAssetIdNode.value.dependencies ? - if (targets.size > 0) { - let deps = [ - // Keep the original dependency - { - asset: null, - dep: graph.addNodeByContentKey(node.id, { + let deps = [ + // Keep the original dependency + { + asset: null, + dep: graph.addNodeByContentKey(node.id, { + ...node, + value: { + ...node.value, + symbols: node.value.symbols + ? new Map( + [...node.value.symbols].filter(([k]) => + externalSymbols.has(k), + ), + ) + : undefined, + }, + usedSymbolsUp: new Map( + [...node.usedSymbolsUp].filter(([k]) => + externalSymbols.has(k), + ), + ), + usedSymbolsDown: new Set(), + excluded: externalSymbols.size === 0, + }), + }, + ...[...targets].map(([asset, target]) => { + let newNodeId = hashString( + node.id + [...target.keys()].join(','), + ); + return { + asset, + dep: graph.addNodeByContentKey(newNodeId, { ...node, - excluded: true, - }), - }, - ...[...targets].map(([asset, target]) => { - let newNodeId = hashString( - node.id + [...target.keys()].join(','), - ); - return { - asset, - dep: graph.addNodeByContentKey(newNodeId, { - ...node, + id: newNodeId, + value: { + ...node.value, id: newNodeId, - value: { - ...node.value, - id: newNodeId, - symbols: node.value.symbols - ? new Map( - [...node.value.symbols].filter( - ([k]) => target.has(k) || k === '*', - ), - ) - : undefined, - }, - usedSymbolsUp: new Map( - [...node.usedSymbolsUp].filter( - ([k]) => target.has(k) || k === '*', - ), + symbols: node.value.symbols + ? new Map( + [...node.value.symbols].filter( + ([k]) => target.has(k) || k === '*', + ), + ) + : undefined, + }, + usedSymbolsUp: new Map( + [...node.usedSymbolsUp].filter( + ([k]) => target.has(k) || k === '*', ), - usedSymbolsDown: new Set(), - symbolTarget: target, - }), - }; - }), - ]; - dependencies.set(nodeId, deps); - } else { - dependencies.set(nodeId, [ - { - asset: null, - dep: graph.addNodeByContentKey(node.id, { - ...node, - // TODO ?? - excluded: !hasExternal, + ), + usedSymbolsDown: new Set(), + symbolTarget: target, }), - }, - ]); - } + }; + }), + ]; + dependencies.set(nodeId, deps); + // } } else { let bundleGraphNodeId = graph.addNodeByContentKey(node.id, node); if (node.id === assetGraphRootNode?.id) { From f9baf0ccb9a456c22464a9b980c10f5478a889c7 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 29 Aug 2022 11:42:46 +0200 Subject: [PATCH 33/48] f --- packages/core/core/src/BundleGraph.js | 92 ++++++++++++++------------- packages/core/test-utils/src/utils.js | 11 +++- 2 files changed, 56 insertions(+), 47 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index dc5215cf6b7..c76fcb237b7 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -179,33 +179,34 @@ export default class BundleGraph { } } - assetGraph.dfs({ - visit: nodeId => { - let node = nullthrows(assetGraph.getNode(nodeId)); - if (node.type === 'asset_group') { - // Don't copy over asset groups into the bundle graph. - return; - } else if (node.type === 'dependency' && node.value.symbols != null) { - // asset -> symbols that should be imported directly from that asset - let targets = new DefaultMap>( - () => new Map(), - ); - let externalSymbols = new Set(); - for (let [symbol, resolvedSymbol] of node.usedSymbolsUp) { - if (resolvedSymbol) { - targets - .get(resolvedSymbol.asset) - .set(symbol, resolvedSymbol.symbol ?? symbol); - } else { - externalSymbols.add(symbol); - } + let walkVisited = new Set(); + function walk(nodeId) { + if (walkVisited.has(nodeId)) return; + walkVisited.add(nodeId); + + let node = nullthrows(assetGraph.getNode(nodeId)); + if (node.type === 'dependency' && node.value.symbols != null) { + // asset -> symbols that should be imported directly from that asset + let targets = new DefaultMap>( + () => new Map(), + ); + let externalSymbols = new Set(); + + for (let [symbol, resolvedSymbol] of node.usedSymbolsUp) { + if (resolvedSymbol) { + targets + .get(resolvedSymbol.asset) + .set(symbol, resolvedSymbol.symbol ?? symbol); + } else { + externalSymbols.add(symbol); } + } - // if (targets.size === 1 && targets.keys().next().value ==) { - // // No special handling - // let bundleGraphNodeId = graph.addNodeByContentKey(node.id, node); - // assetGraphNodeIdToBundleGraphNodeId.set(nodeId, bundleGraphNodeId); - // } else { + // Only perform rewriting when there is an imported symbol + // - If the target is side-effect-free, the symbols point to the actual target and removing + // the original dependency resolution is fine + // - Otherwise, keep this dependency unchanged for its potential side effects + if (node.usedSymbolsUp.size > 0) { // TODO adjust sourceAssetIdNode.value.dependencies ? let deps = [ // Keep the original dependency @@ -264,31 +265,32 @@ export default class BundleGraph { }), ]; dependencies.set(nodeId, deps); - // } + + // Jump to the dependencies that are used in this dependency + for (let id of targets.keys()) { + walk(assetGraph.getNodeIdByContentKey(id)); + } + return; } else { + // No special handling let bundleGraphNodeId = graph.addNodeByContentKey(node.id, node); - if (node.id === assetGraphRootNode?.id) { - graph.setRootNodeId(bundleGraphNodeId); - } assetGraphNodeIdToBundleGraphNodeId.set(nodeId, bundleGraphNodeId); } - }, - startNodeId: null, - getChildren: nodeId => { - let node = nullthrows(assetGraph.getNode(nodeId)); - if (node.type === 'dependency' && node.value.symbols != null) { - // Jump to the dependencies that are used in this dependencies - return [...node.usedSymbolsUp.values()] - .filter(Boolean) - .map(v => assetGraph.getNodeIdByContentKey(v.asset)); - } else { - return assetGraph.getNodeIdsConnectedFrom( - nodeId, - assetGraphEdgeTypes.null, - ); + } + // Don't copy over asset groups into the bundle graph. + else if (node.type !== 'asset_group') { + let bundleGraphNodeId = graph.addNodeByContentKey(node.id, node); + if (node.id === assetGraphRootNode?.id) { + graph.setRootNodeId(bundleGraphNodeId); } - }, - }); + assetGraphNodeIdToBundleGraphNodeId.set(nodeId, bundleGraphNodeId); + } + + for (let id of assetGraph.getNodeIdsConnectedFrom(nodeId)) { + walk(id); + } + } + walk(nullthrows(assetGraph.rootNodeId)); for (let edge of assetGraph.getAllEdges()) { if (assetGroupIds.has(edge.from)) { diff --git a/packages/core/test-utils/src/utils.js b/packages/core/test-utils/src/utils.js index 4642db29dd3..7194396d079 100644 --- a/packages/core/test-utils/src/utils.js +++ b/packages/core/test-utils/src/utils.js @@ -33,6 +33,7 @@ import https from 'https'; import {makeDeferredWithPromise, normalizeSeparators} from '@parcel/utils'; import _chalk from 'chalk'; import resolve from 'resolve'; +import {removePropertiesDeep} from '@babel/types'; export const workerFarm = (createWorkerFarm(): WorkerFarm); export const inputFS: NodeFS = new NodeFS(); @@ -170,9 +171,15 @@ export function findDependency( `Couldn't find asset ${assetFileName}`, ); - let dependency = bundleGraph + let dependencies = bundleGraph .getDependencies(asset) - .find(d => d.specifier === specifier); + .filter(d => d.specifier === specifier); + + let dependency = + dependencies.length > 1 + ? dependencies.find(d => !bundleGraph.isDependencySkipped(d)) + : dependencies[0]; + invariant( dependency != null, `Couldn't find dependency ${assetFileName} -> ${specifier}`, From 2d1590fc898fa8bbf7e77bd9dd869244d2800177 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 29 Aug 2022 12:00:02 +0200 Subject: [PATCH 34/48] f --- packages/core/core/src/BundleGraph.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index c76fcb237b7..0bc9ed4c40d 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -202,11 +202,19 @@ export default class BundleGraph { } } - // Only perform rewriting when there is an imported symbol - // - If the target is side-effect-free, the symbols point to the actual target and removing - // the original dependency resolution is fine - // - Otherwise, keep this dependency unchanged for its potential side effects - if (node.usedSymbolsUp.size > 0) { + if ( + // Only perform rewriting when there is an imported symbol + // - If the target is side-effect-free, the symbols point to the actual target and removing + // the original dependency resolution is fine + // - Otherwise, keep this dependency unchanged for its potential side effects + node.usedSymbolsUp.size > 0 && + // We currently can't replace async imports from + // (parcelRequire("...")).then(({ a }) => a); + // to + // (parcelRequire("...")).then((a)=>a); + // (because of symbolTarget == { a -> * } ) + node.value.priority === Priority.sync + ) { // TODO adjust sourceAssetIdNode.value.dependencies ? let deps = [ // Keep the original dependency From e9aedcf61c429ffe647ae24fe89dff12baa7569a Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 29 Aug 2022 13:33:55 +0200 Subject: [PATCH 35/48] fixup tests --- .../integration-tests/test/scope-hoisting.js | 30 +++++-------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js index dd918b9242d..7757d98dd14 100644 --- a/packages/core/integration-tests/test/scope-hoisting.js +++ b/packages/core/integration-tests/test/scope-hoisting.js @@ -1246,13 +1246,6 @@ describe('scope hoisting', function () { ), ); - assert.deepStrictEqual( - new Set( - b.getUsedSymbols(findDependency(b, 'a.js', './library/index.js')), - ), - new Set(['foo', 'foobar']), - ); - let calls = []; let output = await run(b, { sideEffect: v => { @@ -2694,18 +2687,7 @@ describe('scope hoisting', function () { ), new Set(['borderRadius', 'gridSize']), ); - assert.deepStrictEqual( - new Set( - bundleEvent.bundleGraph.getUsedSymbols( - findDependency( - bundleEvent.bundleGraph, - 'theme.js', - './themeColors', - ), - ), - ), - new Set('*'), - ); + assert(!findAsset(bundleEvent.bundleGraph, 'theme.js')); assert(findAsset(bundleEvent.bundleGraph, 'themeColors.js')); await overlayFS.copyFile( @@ -5461,7 +5443,10 @@ describe('scope hoisting', function () { shouldDisableCache: true, }); - await run(b); + let contents = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8'); + console.log(contents); + + assert.strictEqual(await run(b), 'bar'); await overlayFS.copyFile( path.join(testDir, 'index2.js'), @@ -5474,7 +5459,7 @@ describe('scope hoisting', function () { shouldDisableCache: false, }); - await run(b); + assert.strictEqual(await run(b), 'bar foo'); await overlayFS.copyFile( path.join(testDir, 'index3.js'), @@ -5487,8 +5472,7 @@ describe('scope hoisting', function () { shouldDisableCache: false, }); - let output = await run(b); - assert.strictEqual(output, 'bar foo bar'); + assert.strictEqual(await run(b), 'bar foo bar'); }); it("not insert unused requires that aren't registered anywhere", async function () { From 91b3567ceb47aef0fb933091e1a1f140247830fa Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 29 Aug 2022 13:34:03 +0200 Subject: [PATCH 36/48] Fixups --- packages/core/core/src/BundleGraph.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 0bc9ed4c40d..c0e3fe76c73 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -1549,11 +1549,12 @@ export default class BundleGraph { let symbolLookup = new Map( [...depSymbols].map(([key, val]) => [val.local, key]), ); - let depSymbol = - identifier != null - ? symbolLookup.get(symbolTarget?.get(identifier) ?? identifier) - : undefined; + let depSymbol = symbolLookup.get(identifier); if (depSymbol != null) { + if (symbolTarget != null) { + depSymbol = symbolTarget.get(depSymbol) ?? depSymbol; + } + let resolved = this.getResolvedAsset(dep); if (!resolved || resolved.id === asset.id) { // External module or self-reference From 64dc9a7be83118fbed83120cdc6dd19873b5d880 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 29 Aug 2022 14:52:51 +0200 Subject: [PATCH 37/48] Cleanup --- packages/core/core/src/AssetGraph.js | 16 ++-------------- packages/core/core/src/BundleGraph.js | 6 +----- packages/core/core/src/dumpGraphToGraphViz.js | 9 ++------- .../core/core/src/requests/AssetGraphRequest.js | 10 ++-------- 4 files changed, 7 insertions(+), 34 deletions(-) diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index 72563b13b05..f4131ac392c 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -30,15 +30,6 @@ import {ContentGraph} from '@parcel/graph'; import {createDependency} from './Dependency'; import {type ProjectPath, fromProjectPathRelative} from './projectPath'; -export const assetGraphEdgeTypes = { - null: 1, - // In addition to the null edge, a dependency can be connected to the asset containing the symbols - // that the dependency requested (after reexports were skipped). - redirected: 2, -}; - -export type AssetGraphEdgeType = $Values; - type InitOpts = {| entries?: Array, targets?: Array, @@ -52,7 +43,7 @@ type AssetGraphOpts = {| |}; type SerializedAssetGraph = {| - ...SerializedContentGraph, + ...SerializedContentGraph, hash?: ?string, symbolPropagationRan: boolean, |}; @@ -119,10 +110,7 @@ export function nodeFromEntryFile(entry: Entry): EntryFileNode { }; } -export default class AssetGraph extends ContentGraph< - AssetGraphNode, - AssetGraphEdgeType, -> { +export default class AssetGraph extends ContentGraph { onNodeRemoved: ?(nodeId: NodeId) => mixed; hash: ?string; envCache: Map; diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index c0e3fe76c73..ff2cae180cd 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -38,7 +38,6 @@ import {Priority, BundleBehavior, SpecifierType} from './types'; import {getBundleGroupId, getPublicId} from './utils'; import {ISOLATED_ENVS} from './public/Environment'; import {fromProjectPath} from './projectPath'; -import {assetGraphEdgeTypes} from './AssetGraph'; export const bundleGraphEdgeTypes = { // A lack of an edge type indicates to follow the edge while traversing @@ -172,10 +171,7 @@ export default class BundleGraph { assetPublicIds.add(publicId); } } else if (node.type === 'asset_group') { - assetGroupIds.set( - nodeId, - assetGraph.getNodeIdsConnectedFrom(nodeId, assetGraphEdgeTypes.null), - ); + assetGroupIds.set(nodeId, assetGraph.getNodeIdsConnectedFrom(nodeId)); } } diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index b32d602925c..1581ccfe9e3 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -3,8 +3,6 @@ import type {Asset, BundleBehavior} from '@parcel/types'; import type {Graph} from '@parcel/graph'; import type {AssetGraphNode, BundleGraphNode, Environment} from './types'; -import type {AssetGraphEdgeType} from './AssetGraph'; -import {assetGraphEdgeTypes} from './AssetGraph'; import {bundleGraphEdgeTypes} from './BundleGraph'; import {requestGraphEdgeTypes} from './RequestTracker'; @@ -40,7 +38,7 @@ const TYPE_COLORS = { export default async function dumpGraphToGraphViz( graph: - | Graph + | Graph | Graph<{| assets: Set, sourceBundles: Set, @@ -48,10 +46,7 @@ export default async function dumpGraphToGraphViz( |}> | Graph, name: string, - edgeTypes?: - | typeof assetGraphEdgeTypes - | typeof bundleGraphEdgeTypes - | typeof requestGraphEdgeTypes, + edgeTypes?: typeof bundleGraphEdgeTypes | typeof requestGraphEdgeTypes, ): Promise { if ( process.env.PARCEL_BUILD_ENV === 'production' || diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 5fd3c9bc29c..dd3e66306b0 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -27,7 +27,7 @@ import {hashString} from '@parcel/hash'; import logger from '@parcel/logger'; import ThrowableDiagnostic, {md} from '@parcel/diagnostic'; import {BundleBehavior, Priority} from '../types'; -import AssetGraph, {assetGraphEdgeTypes} from '../AssetGraph'; +import AssetGraph from '../AssetGraph'; import {PARCEL_VERSION} from '../constants'; import createEntryRequest from './EntryRequest'; import createTargetRequest from './TargetRequest'; @@ -211,7 +211,6 @@ export class AssetGraphBuilder { await dumpGraphToGraphViz( this.assetGraph, 'AssetGraph_' + this.name + '_before_prop', - assetGraphEdgeTypes, ); try { this.propagateSymbols(); @@ -219,16 +218,11 @@ export class AssetGraphBuilder { await dumpGraphToGraphViz( this.assetGraph, 'AssetGraph_' + this.name + '_failed', - assetGraphEdgeTypes, ); throw e; } } - await dumpGraphToGraphViz( - this.assetGraph, - 'AssetGraph_' + this.name, - assetGraphEdgeTypes, - ); + await dumpGraphToGraphViz(this.assetGraph, 'AssetGraph_' + this.name); return { assetGraph: this.assetGraph, From 533713dfedc136307d3321a3e794877830b73272 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Wed, 31 Aug 2022 15:04:52 +0200 Subject: [PATCH 38/48] Add test --- .../es6/codesplit-reexports/library/bar.js | 1 + .../es6/codesplit-reexports/library/bar2.js | 1 + .../es6/codesplit-reexports/library/foo.js | 1 + .../es6/codesplit-reexports/library/foo2.js | 2 ++ .../es6/codesplit-reexports/library/index.js | 7 ++++ .../codesplit-reexports/library/package.json | 4 +++ .../es6/codesplit-reexports/src/async.js | 3 ++ .../es6/codesplit-reexports/src/entry.js | 3 ++ .../integration-tests/test/scope-hoisting.js | 33 +++++++++++++++++++ 9 files changed, 55 insertions(+) create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar2.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo2.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/index.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/package.json create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/async.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar.js new file mode 100644 index 00000000000..b8b030fb17b --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar.js @@ -0,0 +1 @@ +export const bar = 3; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar2.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar2.js new file mode 100644 index 00000000000..8a4703d5b88 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar2.js @@ -0,0 +1 @@ +export const bar2 = 30; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo.js new file mode 100644 index 00000000000..cc57a55e257 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo.js @@ -0,0 +1 @@ +export const foo = 2; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo2.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo2.js new file mode 100644 index 00000000000..dc8c6371bf0 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo2.js @@ -0,0 +1,2 @@ +export const foo2 = 20; + diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/index.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/index.js new file mode 100644 index 00000000000..760d6bb9174 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/index.js @@ -0,0 +1,7 @@ +export { foo } from "./foo"; +export * from "./bar"; + +export { foo2 as foo3 } from "./foo2"; +export { bar2 as bar3 } from "./bar2"; + +export const own = 3; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/package.json b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/package.json new file mode 100644 index 00000000000..a2dd4df2af0 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/package.json @@ -0,0 +1,4 @@ +{ + "name": "lib", + "sideEffects": false +} diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/async.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/async.js new file mode 100644 index 00000000000..09e18d5f44e --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/async.js @@ -0,0 +1,3 @@ +import { foo3, bar3 } from "../library"; + +export default [foo3, bar3]; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js new file mode 100644 index 00000000000..d5824a7266e --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js @@ -0,0 +1,3 @@ +import { foo, bar } from "../library"; + +output = import("./async").then(v => [v.default, [foo, bar]]) diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js index 7757d98dd14..a9b120f03b6 100644 --- a/packages/core/integration-tests/test/scope-hoisting.js +++ b/packages/core/integration-tests/test/scope-hoisting.js @@ -2229,6 +2229,39 @@ describe('scope hoisting', function () { assert.deepEqual(output.foo, 'bar'); }); + it('should correctly codesplit even with reexporting library index', async function () { + let b = await bundle( + path.join( + __dirname, + '/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js', + ), + ); + + assertBundles(b, [ + { + type: 'js', + assets: [ + 'entry.js', + 'foo.js', + 'bar.js', + 'bundle-url.js', + 'cacheLoader.js', + 'js-loader.js', + ], + }, + { + type: 'js', + assets: ['async.js', 'foo2.js', 'bar2.js'], + }, + ]); + + let output = await run(b); + assert.deepEqual(output, [ + [20, 30], + [2, 3], + ]); + }); + it('should correctly handle circular dependencies', async function () { let b = await bundle( path.join(__dirname, '/integration/scope-hoisting/es6/circular/a.mjs'), From 9698d0d4b9c079648f15d7b99aa8e8155d9318aa Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 1 Sep 2022 14:15:59 +0200 Subject: [PATCH 39/48] Cleanup --- packages/core/core/src/dumpGraphToGraphViz.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 1581ccfe9e3..782f9192082 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -28,7 +28,6 @@ const TYPE_COLORS = { references: 'red', sibling: 'green', // asset graph - redirected: 'grey', // request graph invalidated_by_create: 'green', invalidated_by_create_above: 'orange', From 0a4c8b330054f4175d4e4279733d37587c23c046 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 1 Sep 2022 14:18:29 +0200 Subject: [PATCH 40/48] Less strict test --- .../core/integration-tests/test/javascript.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js index 348cc87216b..ccda745bbad 100644 --- a/packages/core/integration-tests/test/javascript.js +++ b/packages/core/integration-tests/test/javascript.js @@ -6511,12 +6511,17 @@ describe('javascript', function () { {require: false}, ); - assert.deepEqual( - calls, - shouldScopeHoist - ? ['key', 'foo', 'index'] - : ['key', 'foo', 'bar', 'types', 'index'], - ); + if (shouldScopeHoist) { + try { + assert.deepEqual(calls, ['key', 'foo', 'index']); + } catch (e) { + // A different dependency order, but this is deemed acceptable as it's sideeffect free + assert.deepEqual(calls, ['foo', 'key', 'index']); + } + } else { + assert.deepEqual(calls, ['key', 'foo', 'bar', 'types', 'index']); + } + assert.deepEqual(res.output, ['key', 'foo']); }); From e7deed2059328da9f8daeb91fa8ecd9653195877 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 1 Sep 2022 14:34:32 +0200 Subject: [PATCH 41/48] Cleanup --- packages/core/integration-tests/test/plugin.js | 11 +---------- .../core/integration-tests/test/scope-hoisting.js | 3 --- packages/core/test-utils/src/utils.js | 1 - 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/packages/core/integration-tests/test/plugin.js b/packages/core/integration-tests/test/plugin.js index 7472e28e8f8..ec38c5ce0b2 100644 --- a/packages/core/integration-tests/test/plugin.js +++ b/packages/core/integration-tests/test/plugin.js @@ -8,7 +8,6 @@ import { bundle, distDir, findAsset, - findDependency, outputFS as fs, overlayFS, run, @@ -123,7 +122,7 @@ parcel-transformer-b`, }, ); - assert.deepStrictEqual(!findAsset(b, 'index.js')); + assert(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'a.js')))), new Set(['a']), @@ -132,14 +131,6 @@ parcel-transformer-b`, new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'b.js')))), new Set(['b']), ); - assert.deepStrictEqual( - new Set(b.getUsedSymbols(findDependency(b, 'index.js', './a.js'))), - new Set(['a']), - ); - assert.deepStrictEqual( - new Set(b.getUsedSymbols(findDependency(b, 'index.js', './b.js'))), - new Set(['b']), - ); let calls = []; await run(b, { diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js index a9b120f03b6..857cfce2856 100644 --- a/packages/core/integration-tests/test/scope-hoisting.js +++ b/packages/core/integration-tests/test/scope-hoisting.js @@ -5476,9 +5476,6 @@ describe('scope hoisting', function () { shouldDisableCache: true, }); - let contents = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8'); - console.log(contents); - assert.strictEqual(await run(b), 'bar'); await overlayFS.copyFile( diff --git a/packages/core/test-utils/src/utils.js b/packages/core/test-utils/src/utils.js index 7194396d079..3fadfe85824 100644 --- a/packages/core/test-utils/src/utils.js +++ b/packages/core/test-utils/src/utils.js @@ -33,7 +33,6 @@ import https from 'https'; import {makeDeferredWithPromise, normalizeSeparators} from '@parcel/utils'; import _chalk from 'chalk'; import resolve from 'resolve'; -import {removePropertiesDeep} from '@babel/types'; export const workerFarm = (createWorkerFarm(): WorkerFarm); export const inputFS: NodeFS = new NodeFS(); From 8beca815f4378557e7399d3d24abe56646bb2484 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 1 Sep 2022 14:50:53 +0200 Subject: [PATCH 42/48] TODO comment --- packages/core/core/src/BundleGraph.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index ff2cae180cd..220058ed001 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -204,11 +204,13 @@ export default class BundleGraph { // the original dependency resolution is fine // - Otherwise, keep this dependency unchanged for its potential side effects node.usedSymbolsUp.size > 0 && - // We currently can't replace async imports from + // TODO We currently can't rename imports in async imports, e.g. from // (parcelRequire("...")).then(({ a }) => a); // to + // (parcelRequire("...")).then(({ a: b }) => a); + // or // (parcelRequire("...")).then((a)=>a); - // (because of symbolTarget == { a -> * } ) + // if the reexporting asset did `export {a as b}` or `export * as a` node.value.priority === Priority.sync ) { // TODO adjust sourceAssetIdNode.value.dependencies ? From 8397d4b4dbbeba49be9a206f797c85b3f91b854c Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 1 Sep 2022 15:26:15 +0200 Subject: [PATCH 43/48] Remove symbolTarget --- packages/core/core/src/AssetGraph.js | 1 - packages/core/core/src/BundleGraph.js | 40 ++++------------ packages/core/core/src/dumpGraphToGraphViz.js | 5 -- packages/core/core/src/public/BundleGraph.js | 2 +- packages/core/core/src/public/Symbols.js | 47 ++++--------------- packages/core/core/src/types.js | 3 -- 6 files changed, 17 insertions(+), 81 deletions(-) diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index f4131ac392c..534d7f0b263 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -60,7 +60,6 @@ export function nodeFromDep(dep: Dependency): DependencyNode { usedSymbolsDownDirty: true, usedSymbolsUpDirtyDown: true, usedSymbolsUpDirtyUp: true, - symbolTarget: null, }; } diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 220058ed001..5aeb20c5240 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -253,19 +253,18 @@ export default class BundleGraph { id: newNodeId, symbols: node.value.symbols ? new Map( - [...node.value.symbols].filter( - ([k]) => target.has(k) || k === '*', - ), + [...node.value.symbols] + .filter(([k]) => target.has(k) || k === '*') + .map(([k, v]) => [target.get(k) ?? k, v]), ) : undefined, }, usedSymbolsUp: new Map( - [...node.usedSymbolsUp].filter( - ([k]) => target.has(k) || k === '*', - ), + [...node.usedSymbolsUp] + .filter(([k]) => target.has(k) || k === '*') + .map(([k, v]) => [target.get(k) ?? k, v]), ), usedSymbolsDown: new Set(), - symbolTarget: target, }), }; }), @@ -980,17 +979,6 @@ export default class BundleGraph { }); } - getDependenciesWithSymbolTarget( - asset: Asset, - ): Array<[Dependency, ?Map]> { - let nodeId = this._graph.getNodeIdByContentKey(asset.id); - return this._graph.getNodeIdsConnectedFrom(nodeId).map(id => { - let node = nullthrows(this._graph.getNode(id)); - invariant(node.type === 'dependency'); - return [node.value, node.symbolTarget]; - }); - } - traverseAssets( bundle: Bundle, visit: GraphVisitor, @@ -1535,9 +1523,9 @@ export default class BundleGraph { let found = false; let nonStaticDependency = false; let skipped = false; - let deps = this.getDependenciesWithSymbolTarget(asset).reverse(); + let deps = this.getDependencies(asset).reverse(); let potentialResults = []; - for (let [dep, symbolTarget] of deps) { + for (let dep of deps) { let depSymbols = dep.symbols; if (!depSymbols) { nonStaticDependency = true; @@ -1549,10 +1537,6 @@ export default class BundleGraph { ); let depSymbol = symbolLookup.get(identifier); if (depSymbol != null) { - if (symbolTarget != null) { - depSymbol = symbolTarget.get(depSymbol) ?? depSymbol; - } - let resolved = this.getResolvedAsset(dep); if (!resolved || resolved.id === asset.id) { // External module or self-reference @@ -1882,14 +1866,6 @@ export default class BundleGraph { let node = this._graph.getNodeByContentKey(dep.id); invariant(node && node.type === 'dependency'); let result = new Set(node.usedSymbolsUp.keys()); - if (node.symbolTarget) { - for (let [k, v] of node.symbolTarget) { - if (result.has(k)) { - result.delete(k); - result.add(v); - } - } - } return this._symbolPropagationRan ? makeReadOnlySet(result) : null; } diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 782f9192082..aefbb0cfc1b 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -125,11 +125,6 @@ export default async function dumpGraphToGraphViz( label += '\\nusedSymbolsDown: ' + [...node.usedSymbolsDown].join(','); } - if (node.symbolTarget && node.symbolTarget?.size > 0) { - label += - '\\nsymbolTarget: ' + - [...node.symbolTarget].map(([a, b]) => `${a}:${b}`).join(','); - } } else { label += '\\nsymbols: cleared'; } diff --git a/packages/core/core/src/public/BundleGraph.js b/packages/core/core/src/public/BundleGraph.js index 3f1e07aeaf0..2114da59a14 100644 --- a/packages/core/core/src/public/BundleGraph.js +++ b/packages/core/core/src/public/BundleGraph.js @@ -313,7 +313,7 @@ export default class BundleGraph getSymbols(dep: IDependency): IDependencySymbols { let node = this.#graph._graph.getNodeByContentKey(dep.id); invariant(node && node.type === 'dependency'); - return new DependencySymbols(this.#options, node.value, node.symbolTarget); + return new DependencySymbols(this.#options, node.value); } getEntryRoot(target: Target): FilePath { diff --git a/packages/core/core/src/public/Symbols.js b/packages/core/core/src/public/Symbols.js index 75130c0d885..f426b2cd2ec 100644 --- a/packages/core/core/src/public/Symbols.js +++ b/packages/core/core/src/public/Symbols.js @@ -200,33 +200,21 @@ export class DependencySymbols implements IDependencySymbols { */ #value: Dependency; #options: ParcelOptions; - #symbolTarget: ?Map; - constructor( - options: ParcelOptions, - dep: Dependency, - symbolTarget: ?Map, - ): DependencySymbols { + constructor(options: ParcelOptions, dep: Dependency): DependencySymbols { let existing = valueToDependencySymbols.get(dep); if (existing != null) { return existing; } this.#value = dep; this.#options = options; - this.#symbolTarget = symbolTarget; return this; } - #translateExportSymbol(exportSymbol: ISymbol): ISymbol { - return this.#symbolTarget?.get(exportSymbol) ?? exportSymbol; - } - // immutable: hasExportSymbol(exportSymbol: ISymbol): boolean { - return Boolean( - this.#value.symbols?.has(this.#translateExportSymbol(exportSymbol)), - ); + return Boolean(this.#value.symbols?.has(exportSymbol)); } hasLocalSymbol(local: ISymbol): boolean { @@ -243,9 +231,7 @@ export class DependencySymbols implements IDependencySymbols { ): ?{|local: ISymbol, loc: ?SourceLocation, isWeak: boolean, meta?: ?Meta|} { return fromInternalDependencySymbol( this.#options.projectRoot, - nullthrows(this.#value.symbols).get( - this.#translateExportSymbol(exportSymbol), - ), + nullthrows(this.#value.symbols).get(exportSymbol), ); } @@ -254,32 +240,15 @@ export class DependencySymbols implements IDependencySymbols { } exportSymbols(): Iterable { - let symbols = this.#value.symbols; - if (symbols) { - let result = new Set(); - for (let s of symbols.keys()) { - result.add(this.#translateExportSymbol(s)); - } - return result; - } else { - // $FlowFixMe - return EMPTY_ITERABLE; - } + // $FlowFixMe + return this.#value.symbols ? this.#value.symbols.keys() : EMPTY_ITERABLE; } // $FlowFixMe [Symbol.iterator]() { - let symbols = this.#value.symbols; - if (symbols) { - let result = []; - for (let [s, v] of symbols) { - result.push([this.#translateExportSymbol(s), v]); - } - return result[Symbol.iterator](); - } else { - // $FlowFixMe - return EMPTY_ITERATOR; - } + return this.#value.symbols + ? this.#value.symbols[Symbol.iterator]() + : EMPTY_ITERATOR; } // $FlowFixMe diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index e826f50f52a..daad0225a67 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -317,9 +317,6 @@ export type DependencyNode = {| usedSymbolsUpDirtyDown: boolean, /** dependency was excluded (= no used symbols (globally) & side-effect free) */ excluded: boolean, - /** a dependency importing a single symbol is rewritten to point to the reexported target asset - * instead, this is the name of the export (might have been renamed by reexports) */ - symbolTarget: ?Map, |}; export type RootNode = {|id: ContentKey, +type: 'root', value: string | null|}; From 9da483aef750bed8b5918d261b7e5b926a961321 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 1 Sep 2022 15:27:38 +0200 Subject: [PATCH 44/48] Remove bundleGraph.getSymbols --- packages/core/core/src/public/BundleGraph.js | 8 -------- packages/core/types/index.js | 1 - packages/packagers/css/src/CSSPackager.js | 10 +++------- packages/packagers/js/src/ScopeHoistingPackager.js | 8 ++++---- 4 files changed, 7 insertions(+), 20 deletions(-) diff --git a/packages/core/core/src/public/BundleGraph.js b/packages/core/core/src/public/BundleGraph.js index 2114da59a14..b42a7b68fed 100644 --- a/packages/core/core/src/public/BundleGraph.js +++ b/packages/core/core/src/public/BundleGraph.js @@ -10,7 +10,6 @@ import type { ExportSymbolResolution, FilePath, GraphVisitor, - DependencySymbols as IDependencySymbols, Symbol, SymbolResolution, Target, @@ -28,7 +27,6 @@ import Dependency, {dependencyToInternalDependency} from './Dependency'; import {targetToInternalTarget} from './Target'; import {fromInternalSourceLocation} from '../utils'; import BundleGroup, {bundleGroupToInternalBundleGroup} from './BundleGroup'; -import {DependencySymbols} from './Symbols'; // Friendly access for other modules within this package that need access // to the internal bundle. @@ -310,12 +308,6 @@ export default class BundleGraph } } - getSymbols(dep: IDependency): IDependencySymbols { - let node = this.#graph._graph.getNodeByContentKey(dep.id); - invariant(node && node.type === 'dependency'); - return new DependencySymbols(this.#options, node.value); - } - getEntryRoot(target: Target): FilePath { return this.#graph.getEntryRoot( this.#options.projectRoot, diff --git a/packages/core/types/index.js b/packages/core/types/index.js index feb993bd59e..8dc5c20df5a 100644 --- a/packages/core/types/index.js +++ b/packages/core/types/index.js @@ -1472,7 +1472,6 @@ export interface BundleGraph { * Returns null if symbol propagation didn't run (so the result is unknown). */ getUsedSymbols(Asset | Dependency): ?$ReadOnlySet; - getSymbols(Dependency): DependencySymbols; /** Returns the common root directory for the entry assets of a target. */ getEntryRoot(target: Target): FilePath; } diff --git a/packages/packagers/css/src/CSSPackager.js b/packages/packagers/css/src/CSSPackager.js index eafc307c3dc..c602e9044bf 100644 --- a/packages/packagers/css/src/CSSPackager.js +++ b/packages/packagers/css/src/CSSPackager.js @@ -79,9 +79,7 @@ export default (new Packager({ if (asset.meta.hasReferences) { let replacements = new Map(); for (let dep of asset.getDependencies()) { - for (let [exported, {local}] of bundleGraph.getSymbols( - dep, - )) { + for (let [exported, {local}] of dep.symbols) { let resolved = bundleGraph.getResolvedAsset(dep, bundle); if (resolved) { let resolution = bundleGraph.getSymbolResolution( @@ -218,11 +216,9 @@ async function processCSSModule( let defaultImport = null; if (usedSymbols.has('default')) { let incoming = bundleGraph.getIncomingDependencies(asset); - defaultImport = incoming.find(d => - bundleGraph.getSymbols(d).hasExportSymbol('default'), - ); + defaultImport = incoming.find(d => d.symbols.hasExportSymbol('default')); if (defaultImport) { - let loc = bundleGraph.getSymbols(defaultImport).get('default')?.loc; + let loc = defaultImport.symbols.get('default')?.loc; logger.warn({ message: 'CSS modules cannot be tree shaken when imported with a default specifier', diff --git a/packages/packagers/js/src/ScopeHoistingPackager.js b/packages/packagers/js/src/ScopeHoistingPackager.js index 8153dc5a112..9e0d1755b39 100644 --- a/packages/packagers/js/src/ScopeHoistingPackager.js +++ b/packages/packagers/js/src/ScopeHoistingPackager.js @@ -627,7 +627,7 @@ ${code} continue; } - for (let [imported, {local}] of this.bundleGraph.getSymbols(dep)) { + for (let [imported, {local}] of dep.symbols) { if (local === '*') { continue; } @@ -702,7 +702,7 @@ ${code} this.externals.set(dep.specifier, external); } - for (let [imported, {local}] of this.bundleGraph.getSymbols(dep)) { + for (let [imported, {local}] of dep.symbols) { // If already imported, just add the already renamed variable to the mapping. let renamed = external.get(imported); if (renamed && local !== '*' && replacements) { @@ -1048,7 +1048,7 @@ ${code} let isWrapped = resolved && resolved.meta.shouldWrap; - for (let [imported, {local}] of this.bundleGraph.getSymbols(dep)) { + for (let [imported, {local}] of dep.symbols) { if (imported === '*' && local === '*') { if (!resolved) { // Re-exporting an external module. This should have already been handled in buildReplacements. @@ -1209,7 +1209,7 @@ ${code} dep => this.bundle.hasDependency(dep) && // dep.meta.isES6Module && - this.bundleGraph.getSymbols(dep).hasExportSymbol('default'), + dep.symbols.hasExportSymbol('default'), ); } From ad8d53f903920115b5ce3463933b9959832fa726 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 1 Sep 2022 16:57:38 +0200 Subject: [PATCH 45/48] usedSymbolsUp: ambiguous --- packages/core/core/src/BundleGraph.js | 8 ++- packages/core/core/src/dumpGraphToGraphViz.js | 4 +- .../core/src/requests/AssetGraphRequest.js | 52 ++++++++++++++----- packages/core/core/src/types.js | 12 ++++- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 5aeb20c5240..04dfabbd4bc 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -187,14 +187,18 @@ export default class BundleGraph { () => new Map(), ); let externalSymbols = new Set(); + let hasAmbiguousSymbols = false; for (let [symbol, resolvedSymbol] of node.usedSymbolsUp) { if (resolvedSymbol) { targets .get(resolvedSymbol.asset) .set(symbol, resolvedSymbol.symbol ?? symbol); - } else { + } else if (resolvedSymbol === null) { externalSymbols.add(symbol); + } else if (resolvedSymbol === undefined) { + hasAmbiguousSymbols = true; + break; } } @@ -204,6 +208,8 @@ export default class BundleGraph { // the original dependency resolution is fine // - Otherwise, keep this dependency unchanged for its potential side effects node.usedSymbolsUp.size > 0 && + // Only perform rewriting if the dependency only points to a single asset (e.g. CSS modules) + !hasAmbiguousSymbols && // TODO We currently can't rename imports in async imports, e.g. from // (parcelRequire("...")).then(({ a }) => a); // to diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index aefbb0cfc1b..219a64a034d 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -117,7 +117,9 @@ export default async function dumpGraphToGraphViz( .map(([s, sAsset]) => sAsset ? `${s}(${sAsset.asset}.${sAsset.symbol ?? ''})` - : `${s}(external)`, + : sAsset === null + ? `${s}(external)` + : `${s}(ambiguous)`, ) .join(','); } diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index dd3e66306b0..0c165e9f678 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -551,6 +551,24 @@ export class AssetGraphBuilder { let errors: Array = []; + function usedSymbolsUpAmbiguous(old, current, s, value) { + if (old.has(s)) { + let valueOld = old.get(s); + if ( + valueOld !== value && + !( + valueOld?.asset === value.asset && + valueOld?.symbol === value.symbol + ) + ) { + // The dependency points to multiple assets (via an asset group). + current.set(s, undefined); + return; + } + } + current.set(s, value); + } + for (let incomingDep of incomingDeps) { let incomingDepUsedSymbolsUpOld = incomingDep.usedSymbolsUp; incomingDep.usedSymbolsUp = new Map(); @@ -566,21 +584,31 @@ export class AssetGraphBuilder { s === '*' || assetNode.usedSymbols.has(s) ) { - incomingDep.usedSymbolsUp.set(s, { - asset: assetNode.id, - symbol: s, - }); + usedSymbolsUpAmbiguous( + incomingDepUsedSymbolsUpOld, + incomingDep.usedSymbolsUp, + s, + { + asset: assetNode.id, + symbol: s, + }, + ); } else if (reexportedSymbols.has(s)) { // Forward a reexport only if the current asset is side-effect free and not external let reexport = reexportedSymbols.get(s); - if (!assetNode.value.sideEffects && reexport != null) { - incomingDep.usedSymbolsUp.set(s, reexport); - } else { - incomingDep.usedSymbolsUp.set(s, { - asset: assetNode.id, - symbol: s, - }); - } + let v = + !assetNode.value.sideEffects && reexport != null + ? reexport + : { + asset: assetNode.id, + symbol: s, + }; + usedSymbolsUpAmbiguous( + incomingDepUsedSymbolsUpOld, + incomingDep.usedSymbolsUp, + s, + v, + ); } else if (!hasNamespaceReexport) { let loc = incomingDep.value.symbols?.get(s)?.loc; let [resolutionNodeId] = this.assetGraph.getNodeIdsConnectedFrom( diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index daad0225a67..7ccf54fe14b 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -307,8 +307,16 @@ export type DependencyNode = {| /** dependency was deferred (= no used symbols (in immediate parents) & side-effect free) */ hasDeferred?: boolean, usedSymbolsDown: Set, - // a requested symbol -> the asset it resolved to, and the potentially renamed export name (null if external) - usedSymbolsUp: Map, + /** + * a requested symbol -> either + * - if ambiguous (e.g. dependency to asset group with both CSS modules and JS asset): undefined + * - if external: null + * - the asset it resolved to, and the potentially renamed export name + */ + usedSymbolsUp: Map< + Symbol, + {|asset: ContentKey, symbol: ?Symbol|} | void | null, + >, /** for the "down" pass, the dependency resolution asset needs to be updated */ usedSymbolsDownDirty: boolean, /** for the "up" pass, the parent asset needs to be updated */ From c2895b1ffa0f898b4bcf3c6c24a7434ee8f09175 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 1 Sep 2022 16:59:23 +0200 Subject: [PATCH 46/48] Remove unused DependencySymbols --- packages/core/core/src/public/Symbols.js | 72 ------------------------ packages/core/types/index.js | 23 -------- 2 files changed, 95 deletions(-) diff --git a/packages/core/core/src/public/Symbols.js b/packages/core/core/src/public/Symbols.js index f426b2cd2ec..9d986015485 100644 --- a/packages/core/core/src/public/Symbols.js +++ b/packages/core/core/src/public/Symbols.js @@ -4,7 +4,6 @@ import type { MutableAssetSymbols as IMutableAssetSymbols, AssetSymbols as IAssetSymbols, MutableDependencySymbols as IMutableDependencySymbols, - DependencySymbols as IDependencySymbols, SourceLocation, Meta, } from '@parcel/types'; @@ -192,77 +191,6 @@ export class MutableAssetSymbols implements IMutableAssetSymbols { } } -let valueToDependencySymbols: WeakMap = - new WeakMap(); -export class DependencySymbols implements IDependencySymbols { - /*:: - @@iterator(): Iterator<[ISymbol, {|local: ISymbol, loc: ?SourceLocation, isWeak: boolean, meta?: ?Meta|}]> { return ({}: any); } - */ - #value: Dependency; - #options: ParcelOptions; - - constructor(options: ParcelOptions, dep: Dependency): DependencySymbols { - let existing = valueToDependencySymbols.get(dep); - if (existing != null) { - return existing; - } - this.#value = dep; - this.#options = options; - return this; - } - - // immutable: - - hasExportSymbol(exportSymbol: ISymbol): boolean { - return Boolean(this.#value.symbols?.has(exportSymbol)); - } - - hasLocalSymbol(local: ISymbol): boolean { - if (this.#value.symbols) { - for (let s of this.#value.symbols.values()) { - if (local === s.local) return true; - } - } - return false; - } - - get( - exportSymbol: ISymbol, - ): ?{|local: ISymbol, loc: ?SourceLocation, isWeak: boolean, meta?: ?Meta|} { - return fromInternalDependencySymbol( - this.#options.projectRoot, - nullthrows(this.#value.symbols).get(exportSymbol), - ); - } - - get isCleared(): boolean { - return this.#value.symbols == null; - } - - exportSymbols(): Iterable { - // $FlowFixMe - return this.#value.symbols ? this.#value.symbols.keys() : EMPTY_ITERABLE; - } - - // $FlowFixMe - [Symbol.iterator]() { - return this.#value.symbols - ? this.#value.symbols[Symbol.iterator]() - : EMPTY_ITERATOR; - } - - // $FlowFixMe - [inspect]() { - return `DependencySymbols(${ - this.#value.symbols - ? [...this.#value.symbols] - .map(([s, {local, isWeak}]) => `${s}:${local}${isWeak ? '?' : ''}`) - .join(', ') - : null - })`; - } -} - let valueToMutableDependencySymbols: WeakMap< Dependency, MutableDependencySymbols, diff --git a/packages/core/types/index.js b/packages/core/types/index.js index 8dc5c20df5a..29aa68ee9fd 100644 --- a/packages/core/types/index.js +++ b/packages/core/types/index.js @@ -465,29 +465,6 @@ export interface MutableDependencySymbols // eslint-disable-next-line no-undef delete(exportSymbol: Symbol): void; } -export interface DependencySymbols // eslint-disable-next-line no-undef - extends Iterable< - [ - Symbol, - {|local: Symbol, loc: ?SourceLocation, isWeak: boolean, meta?: ?Meta|}, - ], - > { - /** - * The symbols taht are imports are unknown, rather than just empty. - * This is the default state. - */ - +isCleared: boolean; - get(exportSymbol: Symbol): ?{| - local: Symbol, - loc: ?SourceLocation, - isWeak: boolean, - meta?: ?Meta, - |}; - hasExportSymbol(exportSymbol: Symbol): boolean; - hasLocalSymbol(local: Symbol): boolean; - exportSymbols(): Iterable; -} - export type DependencyPriority = 'sync' | 'parallel' | 'lazy'; export type SpecifierType = 'commonjs' | 'esm' | 'url' | 'custom'; From af3e71642785f4d175a4904a5157fb475e6fea40 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 1 Sep 2022 17:15:34 +0200 Subject: [PATCH 47/48] Cleanup --- .../core/src/requests/AssetGraphRequest.js | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 0c165e9f678..02d46e57d40 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -495,13 +495,7 @@ export class AssetGraphBuilder { assetNode.usedSymbols.add('*'); reexportedSymbols.set(s, {asset: assetNode.id, symbol: s}); } else { - reexportedSymbols.set( - s, - // Forward a reexport only if the current asset is side-effect free. - !assetNode.value.sideEffects - ? sResolved - : {asset: assetNode.id, symbol: s}, - ); + reexportedSymbols.set(s, sResolved); reexportedSymbolsSource.set(s, outgoingDep); } }); @@ -535,13 +529,7 @@ export class AssetGraphBuilder { assetNode.usedSymbols.add('*'); reexportedSymbols.set(s, {asset: assetNode.id, symbol: s}); } else { - reexportedSymbols.set( - s, - // Forward a reexport only if the current asset is side-effect free. - !assetNode.value.sideEffects - ? sResolved - : {asset: assetNode.id, symbol: s}, - ); + reexportedSymbols.set(s, sResolved); reexportedSymbolsSource.set(s, outgoingDep); } }); @@ -594,9 +582,9 @@ export class AssetGraphBuilder { }, ); } else if (reexportedSymbols.has(s)) { - // Forward a reexport only if the current asset is side-effect free and not external let reexport = reexportedSymbols.get(s); let v = + // Forward a reexport only if the current asset is side-effect free and not external !assetNode.value.sideEffects && reexport != null ? reexport : { From c1e5ce0013219a406883645547318c4a282cc67f Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 1 Sep 2022 18:08:49 +0200 Subject: [PATCH 48/48] Fixup SW runtime --- packages/core/integration-tests/test/javascript.js | 2 +- packages/runtimes/service-worker/src/ServiceWorkerRuntime.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js index ccda745bbad..6c25eb10a05 100644 --- a/packages/core/integration-tests/test/javascript.js +++ b/packages/core/integration-tests/test/javascript.js @@ -1684,7 +1684,7 @@ describe('javascript', function () { let bundles = b.getBundles(); let worker = bundles.find(b => b.env.isWorker()); let manifest, version; - await await runBundle(b, worker, { + await runBundle(b, worker, { output(m, v) { manifest = m; version = v; diff --git a/packages/runtimes/service-worker/src/ServiceWorkerRuntime.js b/packages/runtimes/service-worker/src/ServiceWorkerRuntime.js index 90bee96c3d6..2aa01b0b72b 100644 --- a/packages/runtimes/service-worker/src/ServiceWorkerRuntime.js +++ b/packages/runtimes/service-worker/src/ServiceWorkerRuntime.js @@ -11,7 +11,8 @@ export default (new Runtime({ let asset = bundle.traverse((node, _, actions) => { if ( node.type === 'dependency' && - node.value.specifier === '@parcel/service-worker' + node.value.specifier === '@parcel/service-worker' && + !bundleGraph.isDependencySkipped(node.value) ) { actions.stop(); return bundleGraph.getResolvedAsset(node.value, bundle);