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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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/25] 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,