From 71f74d6c8613c477dede6475b27c544115361022 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Sun, 26 Mar 2023 21:26:38 +0200 Subject: [PATCH] Incremental Symbol Propagation (#8723) --- packages/core/core/src/AssetGraph.js | 23 +- packages/core/core/src/BundleGraph.js | 31 +- packages/core/core/src/Dependency.js | 2 +- packages/core/core/src/SymbolPropagation.js | 43 + packages/core/core/src/applyRuntimes.js | 1 + packages/core/core/src/dumpGraphToGraphViz.js | 6 + .../core/src/requests/AssetGraphRequest.js | 1123 ++++++++++------- .../core/src/requests/BundleGraphRequest.js | 14 +- packages/core/core/src/types.js | 17 +- packages/core/core/test/BundleGraph.test.js | 2 + packages/core/core/test/PublicBundle.test.js | 1 - .../test/PublicMutableBundleGraph.test.js | 2 + .../core/core/test/SymbolPropagation.test.js | 716 +++++++++++ .../test/incremental-bundling.js | 49 +- .../index-multi-symbol.js | 2 +- .../a.js | 0 .../node_modules/library/index.js | 3 + .../node_modules/library/other.js | 1 + .../a.js | 4 +- .../b.js | 0 .../node_modules/library/index.js | 3 - .../node_modules/library/other.js | 1 - .../index.3.js | 3 + .../update-used-symbols-remove-export/a.js | 3 + .../update-used-symbols-remove-export/b.1.js | 1 + .../update-used-symbols-remove-export/b.2.js | 1 + .../core/integration-tests/test/javascript.js | 88 +- .../core/integration-tests/test/plugin.js | 1 + .../integration-tests/test/scope-hoisting.js | 213 +++- packages/core/utils/src/collection.js | 9 +- packages/core/utils/src/hash.js | 15 + packages/transformers/js/core/src/hoist.rs | 79 +- packages/transformers/js/src/JSTransformer.js | 21 + .../utils/node-resolver-core/package.json | 2 +- 34 files changed, 1848 insertions(+), 632 deletions(-) create mode 100644 packages/core/core/src/SymbolPropagation.js create mode 100644 packages/core/core/test/SymbolPropagation.test.js rename packages/core/integration-tests/test/integration/{scope-hoisting/es6/unused-import-specifier-node-modules => js-unused-import-specifier-node-modules}/a.js (100%) create mode 100644 packages/core/integration-tests/test/integration/js-unused-import-specifier-node-modules/node_modules/library/index.js create mode 100644 packages/core/integration-tests/test/integration/js-unused-import-specifier-node-modules/node_modules/library/other.js rename packages/core/integration-tests/test/integration/{scope-hoisting/es6/unused-import-specifier => js-unused-import-specifier}/a.js (100%) rename packages/core/integration-tests/test/integration/{scope-hoisting/es6/unused-import-specifier => js-unused-import-specifier}/b.js (100%) delete mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier-node-modules/node_modules/library/index.js delete mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier-node-modules/node_modules/library/other.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-dependency-add/index.3.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/a.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/b.1.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/b.2.js diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index 58544538767..81059f8a461 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -38,14 +38,12 @@ type InitOpts = {| type AssetGraphOpts = {| ...ContentGraphOpts, - symbolPropagationRan: boolean, hash?: ?string, |}; type SerializedAssetGraph = {| ...SerializedContentGraph, hash?: ?string, - symbolPropagationRan: boolean, |}; export function nodeFromDep(dep: Dependency): DependencyNode { @@ -113,15 +111,13 @@ export default class AssetGraph extends ContentGraph { onNodeRemoved: ?(nodeId: NodeId) => mixed; hash: ?string; envCache: Map; - symbolPropagationRan: boolean; safeToIncrementallyBundle: boolean = true; constructor(opts: ?AssetGraphOpts) { if (opts) { - let {hash, symbolPropagationRan, ...rest} = opts; + let {hash, ...rest} = opts; super(rest); this.hash = hash; - this.symbolPropagationRan = symbolPropagationRan; } else { super(); this.setRootNodeId( @@ -133,7 +129,6 @@ export default class AssetGraph extends ContentGraph { ); } this.envCache = new Map(); - this.symbolPropagationRan = false; } // $FlowFixMe[prop-missing] @@ -146,7 +141,6 @@ export default class AssetGraph extends ContentGraph { return { ...super.serialize(), hash: this.hash, - symbolPropagationRan: this.symbolPropagationRan, }; } @@ -199,19 +193,6 @@ export default class AssetGraph extends ContentGraph { removeNode(nodeId: NodeId): void { this.hash = null; this.onNodeRemoved && this.onNodeRemoved(nodeId); - // This needs to mark all connected nodes that doesn't become orphaned - // due to replaceNodesConnectedTo to make sure that the symbols of - // nodes from which at least one parent was removed are updated. - let node = nullthrows(this.getNode(nodeId)); - if (this.isOrphanedNode(nodeId) && node.type === 'dependency') { - let children = this.getNodeIdsConnectedFrom(nodeId).map(id => - nullthrows(this.getNode(id)), - ); - for (let n of children) { - invariant(n.type === 'asset_group' || n.type === 'asset'); - n.usedSymbolsDownDirty = true; - } - } return super.removeNode(nodeId); } @@ -258,6 +239,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.usedSymbolsUp.set('*', undefined); } return node; }); @@ -526,6 +508,7 @@ export default class AssetGraph extends ContentGraph { depNodeIds.push(this.addNode(depNode)); } + assetNode.usedSymbolsUpDirty = true; assetNode.usedSymbolsDownDirty = true; this.replaceNodeIdsConnectedTo( this.getNodeIdByContentKey(assetNode.id), diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 39a020e07c7..91b69304e44 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -29,7 +29,6 @@ import type { } from './types'; import type AssetGraph from './AssetGraph'; import type {ProjectPath} from './projectPath'; -import {nodeFromAsset} from './AssetGraph'; import assert from 'assert'; import invariant from 'assert'; @@ -88,7 +87,6 @@ type BundleGraphOpts = {| bundleContentHashes: Map, assetPublicIds: Set, publicIdByAssetId: Map, - symbolPropagationRan: boolean, |}; type SerializedBundleGraph = {| @@ -97,7 +95,6 @@ type SerializedBundleGraph = {| bundleContentHashes: Map, assetPublicIds: Set, publicIdByAssetId: Map, - symbolPropagationRan: boolean, |}; function makeReadOnlySet(set: Set): $ReadOnlySet { @@ -137,7 +134,6 @@ export default class BundleGraph { _targetEntryRoots: Map = new Map(); /** The internal core Graph structure */ _graph: ContentGraph; - _symbolPropagationRan /*: boolean*/; _bundlePublicIds /*: Set */ = new Set(); constructor({ @@ -145,19 +141,16 @@ export default class BundleGraph { publicIdByAssetId, assetPublicIds, bundleContentHashes, - symbolPropagationRan, }: {| graph: ContentGraph, publicIdByAssetId: Map, assetPublicIds: Set, bundleContentHashes: Map, - symbolPropagationRan: boolean, |}) { this._graph = graph; this._assetPublicIds = assetPublicIds; this._publicIdByAssetId = publicIdByAssetId; this._bundleContentHashes = bundleContentHashes; - this._symbolPropagationRan = symbolPropagationRan; } /** @@ -166,6 +159,7 @@ export default class BundleGraph { */ static fromAssetGraph( assetGraph: AssetGraph, + isProduction: boolean, publicIdByAssetId: Map = new Map(), assetPublicIds: Set = new Set(), ): BundleGraph { @@ -207,7 +201,9 @@ export default class BundleGraph { if ( node.type === 'dependency' && node.value.symbols != null && - node.value.env.shouldScopeHoist + node.value.env.shouldScopeHoist && + // Disable in dev mode because this feature is at odds with safeToIncrementallyBundle + isProduction ) { // asset -> symbols that should be imported directly from that asset let targets = new DefaultMap>( @@ -384,7 +380,6 @@ export default class BundleGraph { assetPublicIds, bundleContentHashes: new Map(), publicIdByAssetId, - symbolPropagationRan: assetGraph.symbolPropagationRan, }); } @@ -395,7 +390,6 @@ export default class BundleGraph { assetPublicIds: this._assetPublicIds, bundleContentHashes: this._bundleContentHashes, publicIdByAssetId: this._publicIdByAssetId, - symbolPropagationRan: this._symbolPropagationRan, }; } @@ -405,7 +399,6 @@ export default class BundleGraph { assetPublicIds: serialized.assetPublicIds, bundleContentHashes: serialized.bundleContentHashes, publicIdByAssetId: serialized.publicIdByAssetId, - symbolPropagationRan: serialized.symbolPropagationRan, }); } @@ -1994,7 +1987,7 @@ export default class BundleGraph { getUsedSymbolsAsset(asset: Asset): ?$ReadOnlySet { let node = this._graph.getNodeByContentKey(asset.id); invariant(node && node.type === 'asset'); - return this._symbolPropagationRan + return node.value.symbols ? makeReadOnlySet(new Set(node.usedSymbols.keys())) : null; } @@ -2002,8 +1995,9 @@ export default class BundleGraph { getUsedSymbolsDependency(dep: Dependency): ?$ReadOnlySet { let node = this._graph.getNodeByContentKey(dep.id); invariant(node && node.type === 'dependency'); - let result = new Set(node.usedSymbolsUp.keys()); - return this._symbolPropagationRan ? makeReadOnlySet(result) : null; + return node.value.symbols + ? makeReadOnlySet(new Set(node.usedSymbolsUp.keys())) + : null; } merge(other: BundleGraph) { @@ -2069,12 +2063,9 @@ export default class BundleGraph { /** * Update the asset in a Bundle Graph and clear the associated Bundle hash. */ - updateAsset(asset: Asset) { - this._graph.updateNode( - this._graph.getNodeIdByContentKey(asset.id), - nodeFromAsset(asset), - ); - let bundles = this.getBundlesWithAsset(asset); + updateAsset(asset: AssetNode) { + this._graph.updateNode(this._graph.getNodeIdByContentKey(asset.id), asset); + let bundles = this.getBundlesWithAsset(asset.value); for (let bundle of bundles) { // the bundle content will change with a modified asset this._bundleContentHashes.delete(bundle.id); diff --git a/packages/core/core/src/Dependency.js b/packages/core/core/src/Dependency.js index 35dd2bc591d..850b243b91a 100644 --- a/packages/core/core/src/Dependency.js +++ b/packages/core/core/src/Dependency.js @@ -38,7 +38,7 @@ type DependencyOpts = {| resolveFrom?: FilePath, range?: SemverRange, target?: Target, - symbols?: Map< + symbols?: ?Map< Symbol, {|local: Symbol, loc: ?SourceLocation, isWeak: boolean, meta?: ?Meta|}, >, diff --git a/packages/core/core/src/SymbolPropagation.js b/packages/core/core/src/SymbolPropagation.js new file mode 100644 index 00000000000..51def5ab7cd --- /dev/null +++ b/packages/core/core/src/SymbolPropagation.js @@ -0,0 +1,43 @@ +// @flow + +import type {Diagnostic} from '@parcel/diagnostic'; +import type {NodeId} from '@parcel/graph'; +import type {ParcelOptions} from './types'; + +import {type default as AssetGraph} from './AssetGraph'; +import {AssetGraphBuilder} from './requests/AssetGraphRequest'; + +export function propagateSymbols({ + options, + assetGraph, + changedAssetsPropagation, + assetGroupsWithRemovedParents, + previousErrors, +}: {| + options: ParcelOptions, + assetGraph: AssetGraph, + changedAssetsPropagation: Set, + assetGroupsWithRemovedParents: Set, + previousErrors?: ?Map>, +|}): Map> { + // TODO move functions from AssetGraphRequest to here + + let builder = new AssetGraphBuilder({ + input: ({} /*: any */), + farm: ({} /*: any */), + invalidateReason: ({} /*: any */), + // $FlowFixMe + api: {getInvalidSubRequests: () => []}, + options, + }); + builder.assetGraph = assetGraph; + builder.changedAssetsPropagation = changedAssetsPropagation; + + return builder.propagateSymbols({ + options: builder.options, + assetGraph: builder.assetGraph, + changedAssetsPropagation: builder.changedAssetsPropagation, + assetGroupsWithRemovedParents: assetGroupsWithRemovedParents, + previousErrors: previousErrors, + }); +} diff --git a/packages/core/core/src/applyRuntimes.js b/packages/core/core/src/applyRuntimes.js index 9a4282de0b3..1d4cce562fc 100644 --- a/packages/core/core/src/applyRuntimes.js +++ b/packages/core/core/src/applyRuntimes.js @@ -183,6 +183,7 @@ export default async function applyRuntimes({ let runtimesGraph = InternalBundleGraph.fromAssetGraph( runtimesAssetGraph, + options.mode === 'production', bundleGraph._publicIdByAssetId, bundleGraph._assetPublicIds, ); diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 8fe6c7bdb3e..b509a61991b 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -127,6 +127,10 @@ export default async function dumpGraphToGraphViz( label += '\\nusedSymbolsDown: ' + [...node.usedSymbolsDown].join(','); } + // if (node.usedSymbolsDownDirty) label += '\\nusedSymbolsDownDirty'; + // if (node.usedSymbolsUpDirtyDown) + // label += '\\nusedSymbolsUpDirtyDown'; + // if (node.usedSymbolsUpDirtyUp) label += '\\nusedSymbolsUpDirtyUp'; } else { label += '\\nsymbols: cleared'; } @@ -149,6 +153,8 @@ export default async function dumpGraphToGraphViz( if (node.usedSymbols.size) { label += '\\nusedSymbols: ' + [...node.usedSymbols].join(','); } + // if (node.usedSymbolsDownDirty) label += '\\nusedSymbolsDownDirty'; + // if (node.usedSymbolsUpDirty) label += '\\nusedSymbolsUpDirty'; } else { label += '\\nsymbols: cleared'; } diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index 2cad34ca891..4c217319916 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -1,8 +1,7 @@ // @flow strict-local -import type {Diagnostic} from '@parcel/diagnostic'; import type {ContentKey, NodeId} from '@parcel/graph'; -import type {Async, Symbol, Meta} from '@parcel/types'; +import type {Async, Meta, Symbol} from '@parcel/types'; import type {SharedReference} from '@parcel/workers'; import type { Asset, @@ -19,10 +18,11 @@ import type { import type {StaticRunOpts, RunAPI} from '../RequestTracker'; import type {EntryResult} from './EntryRequest'; import type {PathRequestInput} from './PathRequest'; +import type {Diagnostic} from '@parcel/diagnostic'; import invariant from 'assert'; import nullthrows from 'nullthrows'; -import {PromiseQueue} from '@parcel/utils'; +import {PromiseQueue, setEqual} from '@parcel/utils'; import {hashString} from '@parcel/hash'; import logger from '@parcel/logger'; import ThrowableDiagnostic, {md} from '@parcel/diagnostic'; @@ -51,7 +51,12 @@ type AssetGraphRequestInput = {| type AssetGraphRequestResult = {| assetGraph: AssetGraph, + /** Assets added/modified since the last successful build. */ changedAssets: Map, + /** Assets added/modified since the last symbol propagation invocation. */ + changedAssetsPropagation: Set, + assetGroupsWithRemovedParents: ?Set, + previousSymbolPropagationErrors: ?Map>, assetRequests: Array, |}; @@ -105,7 +110,8 @@ export class AssetGraphBuilder { assetGraph: AssetGraph; assetRequests: Array = []; queue: PromiseQueue; - changedAssets: Map = new Map(); + changedAssets: Map; + changedAssetsPropagation: Set; optionsRef: SharedReference; options: ParcelOptions; api: RunAPI; @@ -114,6 +120,8 @@ export class AssetGraphBuilder { shouldBuildLazily: boolean; requestedAssetIds: Set; isSingleChangeRebuild: boolean; + assetGroupsWithRemovedParents: Set; + previousSymbolPropagationErrors: Map>; constructor( {input, api, options}: RunInput, @@ -133,6 +141,13 @@ export class AssetGraphBuilder { entries, assetGroups, }); + this.assetGroupsWithRemovedParents = + prevResult?.assetGroupsWithRemovedParents ?? new Set(); + this.previousSymbolPropagationErrors = + prevResult?.previousSymbolPropagationErrors ?? new Map(); + this.changedAssets = prevResult?.changedAssets ?? new Map(); + this.changedAssetsPropagation = + prevResult?.changedAssetsPropagation ?? new Set(); this.assetGraph = assetGraph; this.optionsRef = optionsRef; this.options = options; @@ -148,6 +163,26 @@ export class AssetGraphBuilder { api.getInvalidSubRequests().filter(req => req.type === 'asset_request') .length === 1; this.queue = new PromiseQueue(); + + assetGraph.onNodeRemoved = nodeId => { + this.assetGroupsWithRemovedParents.delete(nodeId); + + // This needs to mark all connected nodes that doesn't become orphaned + // due to replaceNodesConnectedTo to make sure that the symbols of + // nodes from which at least one parent was removed are updated. + let node = nullthrows(assetGraph.getNode(nodeId)); + if (assetGraph.isOrphanedNode(nodeId) && node.type === 'dependency') { + let children = assetGraph.getNodeIdsConnectedFrom(nodeId); + for (let child of children) { + let childNode = nullthrows(assetGraph.getNode(child)); + invariant( + childNode.type === 'asset_group' || childNode.type === 'asset', + ); + childNode.usedSymbolsDownDirty = true; + this.assetGroupsWithRemovedParents.add(child); + } + } + }; } async build(): Promise { @@ -189,43 +224,57 @@ export class AssetGraphBuilder { visit(rootNodeId); await this.queue.run(); - this.api.storeResult( - { - assetGraph: this.assetGraph, - changedAssets: new Map(), - assetRequests: [], - }, - this.cacheKey, - ); - if (errors.length) { - throw errors[0]; // TODO: eventually support multiple errors since requests could reject in parallel - } - // Skip symbol propagation if no target is using scope hoisting - // (mainly for faster development builds) - let entryDependencies = this.assetGraph - .getNodeIdsConnectedFrom(rootNodeId) - .flatMap(entrySpecifier => - this.assetGraph.getNodeIdsConnectedFrom(entrySpecifier), - ) - .flatMap(entryFile => - this.assetGraph.getNodeIdsConnectedFrom(entryFile).map(depNodeId => { - let dep = nullthrows(this.assetGraph.getNode(depNodeId)); - invariant(dep.type === 'dependency'); - return dep; - }), + this.api.storeResult( + { + assetGraph: this.assetGraph, + changedAssets: this.changedAssets, + changedAssetsPropagation: this.changedAssetsPropagation, + assetGroupsWithRemovedParents: this.assetGroupsWithRemovedParents, + previousSymbolPropagationErrors: undefined, + assetRequests: [], + }, + this.cacheKey, ); - this.assetGraph.symbolPropagationRan = entryDependencies.some( - d => d.value.env.shouldScopeHoist, - ); - if (this.assetGraph.symbolPropagationRan) { + // TODO: eventually support multiple errors since requests could reject in parallel + throw errors[0]; + } + + if (this.assetGraph.nodes.size > 1) { await dumpGraphToGraphViz( this.assetGraph, 'AssetGraph_' + this.name + '_before_prop', ); try { - this.propagateSymbols(); + let errors = this.propagateSymbols({ + options: this.options, + assetGraph: this.assetGraph, + changedAssetsPropagation: this.changedAssetsPropagation, + assetGroupsWithRemovedParents: this.assetGroupsWithRemovedParents, + previousErrors: this.previousSymbolPropagationErrors, + }); + this.changedAssetsPropagation.clear(); + + if (errors.size > 0) { + this.api.storeResult( + { + assetGraph: this.assetGraph, + changedAssets: this.changedAssets, + changedAssetsPropagation: this.changedAssetsPropagation, + assetGroupsWithRemovedParents: this.assetGroupsWithRemovedParents, + previousSymbolPropagationErrors: errors, + assetRequests: [], + }, + this.cacheKey, + ); + + // Just throw the first error. Since errors can bubble (e.g. reexporting a reexported symbol also fails), + // determining which failing export is the root cause is nontrivial (because of circular dependencies). + throw new ThrowableDiagnostic({ + diagnostic: [...errors.values()][0], + }); + } } catch (e) { await dumpGraphToGraphViz( this.assetGraph, @@ -236,9 +285,24 @@ export class AssetGraphBuilder { } await dumpGraphToGraphViz(this.assetGraph, 'AssetGraph_' + this.name); + this.api.storeResult( + { + assetGraph: this.assetGraph, + changedAssets: new Map(), + changedAssetsPropagation: this.changedAssetsPropagation, + assetGroupsWithRemovedParents: undefined, + previousSymbolPropagationErrors: undefined, + assetRequests: [], + }, + this.cacheKey, + ); + return { assetGraph: this.assetGraph, changedAssets: this.changedAssets, + changedAssetsPropagation: this.changedAssetsPropagation, + assetGroupsWithRemovedParents: undefined, + previousSymbolPropagationErrors: undefined, assetRequests: this.assetRequests, }; } @@ -278,153 +342,213 @@ export class AssetGraphBuilder { return this.assetGraph.shouldVisitChild(nodeId, childNodeId); } - propagateSymbols() { - // Keep track of dependencies that have changes to their used symbols, - // so we can sort them after propagation. + propagateSymbols({ + options, + assetGraph, + changedAssetsPropagation, + assetGroupsWithRemovedParents, + previousErrors, + }: {| + options: ParcelOptions, + assetGraph: AssetGraph, + changedAssetsPropagation: Set, + assetGroupsWithRemovedParents: Set, + previousErrors?: ?Map>, + |}): Map> { + let changedAssets = new Set( + [...changedAssetsPropagation].map(id => + assetGraph.getNodeIdByContentKey(id), + ), + ); + + // To reorder once at the end let changedDeps = new Set(); + // For the down traversal, the nodes with `usedSymbolsDownDirty = true` are exactly + // `changedAssetsPropagation` (= asset and therefore potentially dependencies changed) or the + // asset children of `assetGroupsWithRemovedParents` (= fewer incoming dependencies causing less + // used symbols). + // + // The up traversal has to consider all nodes that changed in the down traversal + // (`useSymbolsUpDirtyDown = true`) which are listed in `changedDepsUsedSymbolsUpDirtyDown` + // (more or less requested symbols) and in `changedAssetsPropagation` (changing an asset might + // change exports). + + // The dependencies that changed in the down traversal causing an update in the up traversal. + let changedDepsUsedSymbolsUpDirtyDown = new Set(); + // Propagate the requested symbols down from the root to the leaves - this.propagateSymbolsDown((assetNode, incomingDeps, outgoingDeps) => { - if (!assetNode.value.symbols) return; - - // exportSymbol -> identifier - let assetSymbols: $ReadOnlyMap< - Symbol, - {|local: Symbol, loc: ?InternalSourceLocation, meta?: ?Meta|}, - > = assetNode.value.symbols; - // identifier -> exportSymbol - let assetSymbolsInverse; - assetSymbolsInverse = new Map>(); - for (let [s, {local}] of assetSymbols) { - let set = assetSymbolsInverse.get(local); - - if (!set) { - set = new Set(); - assetSymbolsInverse.set(local, set); + this.propagateSymbolsDown( + assetGraph, + changedAssets, + assetGroupsWithRemovedParents, + (assetNode, incomingDeps, outgoingDeps) => { + // exportSymbol -> identifier + let assetSymbols: ?$ReadOnlyMap< + Symbol, + {|local: Symbol, loc: ?InternalSourceLocation, meta?: ?Meta|}, + > = assetNode.value.symbols; + // identifier -> exportSymbol + let assetSymbolsInverse; + if (assetSymbols) { + assetSymbolsInverse = new Map>(); + for (let [s, {local}] of assetSymbols) { + let set = assetSymbolsInverse.get(local); + + if (!set) { + set = new Set(); + assetSymbolsInverse.set(local, set); + } + set.add(s); + } } - set.add(s); - } - let hasNamespaceOutgoingDeps = outgoingDeps.some( - d => d.value.symbols?.get('*')?.local === '*', - ); + let hasNamespaceOutgoingDeps = outgoingDeps.some( + d => d.value.symbols?.get('*')?.local === '*', + ); - // 1) Determine what the incomingDeps requests from the asset - // ---------------------------------------------------------- + // 1) Determine what the incomingDeps requests from the asset + // ---------------------------------------------------------- - let isEntry = false; + let isEntry = false; + let addAll = false; - // Used symbols that are exported or reexported (symbol will be removed again later) by asset. - assetNode.usedSymbols = new Set(); + // Used symbols that are exported or reexported (symbol will be removed again later) by asset. + assetNode.usedSymbols = new Set(); - // Symbols that have to be namespace reexported by outgoingDeps. - let namespaceReexportedSymbols = new Set(); + // Symbols that have to be namespace reexported by outgoingDeps. + let namespaceReexportedSymbols = new Set(); - if (incomingDeps.length === 0) { - // Root in the runtimes Graph - assetNode.usedSymbols.add('*'); - namespaceReexportedSymbols.add('*'); - } else { - for (let incomingDep of incomingDeps) { - if (incomingDep.value.symbols == null) { - isEntry = true; - continue; - } - - for (let exportSymbol of incomingDep.usedSymbolsDown) { - if (exportSymbol === '*') { - assetNode.usedSymbols.add('*'); - namespaceReexportedSymbols.add('*'); - } - if ( - !assetSymbols || - assetSymbols.has(exportSymbol) || - assetSymbols.has('*') - ) { - // An own symbol or a non-namespace reexport - assetNode.usedSymbols.add(exportSymbol); + if (incomingDeps.length === 0) { + // Root in the runtimes Graph + assetNode.usedSymbols.add('*'); + namespaceReexportedSymbols.add('*'); + } else { + for (let incomingDep of incomingDeps) { + if (incomingDep.value.symbols == null) { + if (incomingDep.value.sourceAssetId == null) { + // The root dependency on non-library builds + isEntry = true; + } else { + // A regular dependency with cleared symbols + addAll = true; + } + continue; } - // 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); + + for (let exportSymbol of incomingDep.usedSymbolsDown) { + if (exportSymbol === '*') { + assetNode.usedSymbols.add('*'); + namespaceReexportedSymbols.add('*'); + } + if ( + !assetSymbols || + assetSymbols.has(exportSymbol) || + assetSymbols.has('*') + ) { + // An own symbol or a non-namespace reexport + 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') { + namespaceReexportedSymbols.add(exportSymbol); + } } } } - } - // 2) Distribute the symbols to the outgoing dependencies - // ---------------------------------------------------------- - for (let dep of outgoingDeps) { - let depUsedSymbolsDownOld = dep.usedSymbolsDown; - let depUsedSymbolsDown = new Set(); - dep.usedSymbolsDown = depUsedSymbolsDown; - if ( - assetNode.value.sideEffects || - // For entries, we still need to add dep.value.symbols of the entry (which are "used" but not according to the symbols data) - isEntry || - // If not a single asset is used, we can say the entire subgraph is not used. - // This is e.g. needed when some symbol is imported and then used for a export which isn't used (= "semi-weak" reexport) - // index.js: `import {bar} from "./lib"; ...` - // lib/index.js: `export * from "./foo.js"; export * from "./bar.js";` - // lib/foo.js: `import { data } from "./bar.js"; export const foo = data + " esm2";` - assetNode.usedSymbols.size > 0 || - namespaceReexportedSymbols.size > 0 - ) { - let depSymbols = dep.value.symbols; - if (!depSymbols) continue; + // Incomding dependency with cleared symbols, add everything + if (addAll) { + assetSymbols?.forEach((_, exportSymbol) => + assetNode.usedSymbols.add(exportSymbol), + ); + } + + // 2) Distribute the symbols to the outgoing dependencies + // ---------------------------------------------------------- + for (let dep of outgoingDeps) { + let depUsedSymbolsDownOld = dep.usedSymbolsDown; + let depUsedSymbolsDown = new Set(); + dep.usedSymbolsDown = depUsedSymbolsDown; + if ( + assetNode.value.sideEffects || + // Incoming dependency with cleared symbols + addAll || + // For entries, we still need to add dep.value.symbols of the entry (which are "used" but not according to the symbols data) + isEntry || + // If not a single symbol is used, we can say the entire subgraph is not used. + // This is e.g. needed when some symbol is imported and then used for a export which isn't used (= "semi-weak" reexport) + // index.js: `import {bar} from "./lib"; ...` + // lib/index.js: `export * from "./foo.js"; export * from "./bar.js";` + // lib/foo.js: `import { data } from "./bar.js"; export const foo = data + " esm2";` + assetNode.usedSymbols.size > 0 || + namespaceReexportedSymbols.size > 0 + ) { + let depSymbols = dep.value.symbols; + if (!depSymbols) continue; - if (depSymbols.get('*')?.local === '*') { - for (let s of namespaceReexportedSymbols) { - // We need to propagate the namespaceReexportedSymbols to all namespace dependencies (= even wrong ones because we don't know yet) - depUsedSymbolsDown.add(s); + if (depSymbols.get('*')?.local === '*') { + if (addAll) { + depUsedSymbolsDown.add('*'); + } else { + for (let s of namespaceReexportedSymbols) { + // We need to propagate the namespaceReexportedSymbols to all namespace dependencies (= even wrong ones because we don't know yet) + depUsedSymbolsDown.add(s); + } + } } - } - for (let [symbol, {local}] of depSymbols) { - // Was already handled above - if (local === '*') continue; + for (let [symbol, {local}] of depSymbols) { + // Was already handled above + if (local === '*') continue; - if (!assetSymbolsInverse || !depSymbols.get(symbol)?.isWeak) { - // Bailout or non-weak symbol (= used in the asset itself = not a reexport) - depUsedSymbolsDown.add(symbol); - } else { - let reexportedExportSymbols = assetSymbolsInverse.get(local); - if (reexportedExportSymbols == null) { - // not reexported = used in asset itself + if (!assetSymbolsInverse || !depSymbols.get(symbol)?.isWeak) { + // Bailout or non-weak symbol (= used in the asset itself = not a reexport) depUsedSymbolsDown.add(symbol); - } else if (assetNode.usedSymbols.has('*')) { - // we need everything - depUsedSymbolsDown.add(symbol); - - [...reexportedExportSymbols].forEach(s => - assetNode.usedSymbols.delete(s), - ); } else { - let usedReexportedExportSymbols = [ - ...reexportedExportSymbols, - ].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 + let reexportedExportSymbols = assetSymbolsInverse.get(local); + if (reexportedExportSymbols == null) { + // not reexported = used in asset itself + depUsedSymbolsDown.add(symbol); + } else if (assetNode.usedSymbols.has('*')) { + // we need everything depUsedSymbolsDown.add(symbol); - usedReexportedExportSymbols.forEach(s => + [...reexportedExportSymbols].forEach(s => assetNode.usedSymbols.delete(s), ); + } else { + let usedReexportedExportSymbols = [ + ...reexportedExportSymbols, + ].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); + + usedReexportedExportSymbols.forEach(s => + assetNode.usedSymbols.delete(s), + ); + } } } } + } else { + depUsedSymbolsDown.clear(); + } + if (!setEqual(depUsedSymbolsDownOld, depUsedSymbolsDown)) { + dep.usedSymbolsDownDirty = true; + dep.usedSymbolsUpDirtyDown = true; + changedDepsUsedSymbolsUpDirtyDown.add(dep.id); + } + if (dep.usedSymbolsUpDirtyDown) { + // Set on node creation + changedDepsUsedSymbolsUpDirtyDown.add(dep.id); } - } else { - depUsedSymbolsDown.clear(); - } - if (!equalSet(depUsedSymbolsDownOld, depUsedSymbolsDown)) { - dep.usedSymbolsDownDirty = true; - dep.usedSymbolsUpDirtyDown = true; } - } - }); + }, + ); const logFallbackNamespaceInsertion = ( assetNode, @@ -432,7 +556,7 @@ export class AssetGraphBuilder { depNode1, depNode2, ) => { - if (this.options.logLevel === 'verbose') { + if (options.logLevel === 'verbose') { logger.warn({ message: `${fromProjectPathRelative( assetNode.value.filePath, @@ -448,94 +572,62 @@ export class AssetGraphBuilder { // 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) => { - invariant(assetNode.type === 'asset'); - - let assetSymbols: ?$ReadOnlyMap< - Symbol, - {|local: Symbol, loc: ?InternalSourceLocation, meta?: ?Meta|}, - > = assetNode.value.symbols; - - let assetSymbolsInverse = null; - if (assetSymbols) { - assetSymbolsInverse = new Map>(); - for (let [s, {local}] of assetSymbols) { - let set = assetSymbolsInverse.get(local); - if (!set) { - set = new Set(); - assetSymbolsInverse.set(local, set); - } - set.add(s); - } - } - - // 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 - let reexportedSymbolsSource = new Map(); - for (let outgoingDep of outgoingDeps) { - let outgoingDepSymbols = outgoingDep.value.symbols; - if (!outgoingDepSymbols) continue; - - let isExcluded = - this.assetGraph.getNodeIdsConnectedFrom( - this.assetGraph.getNodeIdByContentKey(outgoingDep.id), - ).length === 0; - // excluded, assume everything that is requested exists - if (isExcluded) { - outgoingDep.usedSymbolsDown.forEach((_, s) => - outgoingDep.usedSymbolsUp.set(s, null), - ); - } - - if (outgoingDepSymbols.get('*')?.local === '*') { - outgoingDep.usedSymbolsUp.forEach((sResolved, s) => { - if (s === 'default') { - return; + let errors = this.propagateSymbolsUp( + assetGraph, + changedAssets, + changedDepsUsedSymbolsUpDirtyDown, + previousErrors, + (assetNode, incomingDeps, outgoingDeps) => { + let assetSymbols: ?$ReadOnlyMap< + Symbol, + {|local: Symbol, loc: ?InternalSourceLocation, meta?: ?Meta|}, + > = assetNode.value.symbols; + + let assetSymbolsInverse = null; + if (assetSymbols) { + assetSymbolsInverse = new Map>(); + for (let [s, {local}] of assetSymbols) { + let set = assetSymbolsInverse.get(local); + if (!set) { + set = new Set(); + assetSymbolsInverse.set(local, set); } - - // 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)) { - if (!assetNode.usedSymbols.has('*')) { - logFallbackNamespaceInsertion( - assetNode, - s, - nullthrows(reexportedSymbolsSource.get(s)), - outgoingDep, - ); - } - assetNode.usedSymbols.add('*'); - reexportedSymbols.set(s, {asset: assetNode.id, symbol: s}); - } else { - reexportedSymbols.set(s, sResolved); - reexportedSymbolsSource.set(s, outgoingDep); - } - }); + set.add(s); + } } - for (let [s, sResolved] of outgoingDep.usedSymbolsUp) { - if (!outgoingDep.usedSymbolsDown.has(s)) { - // usedSymbolsDown is a superset of usedSymbolsUp - continue; + // 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 + let reexportedSymbolsSource = new Map(); + for (let outgoingDep of outgoingDeps) { + let outgoingDepSymbols = outgoingDep.value.symbols; + if (!outgoingDepSymbols) continue; + + let isExcluded = + assetGraph.getNodeIdsConnectedFrom( + assetGraph.getNodeIdByContentKey(outgoingDep.id), + ).length === 0; + // excluded, assume everything that is requested exists + if (isExcluded) { + outgoingDep.usedSymbolsDown.forEach((_, s) => + outgoingDep.usedSymbolsUp.set(s, null), + ); } - let local = outgoingDepSymbols.get(s)?.local; - - if (local == null) { - // Caused by '*' => '*', already handled - continue; - } + if (outgoingDepSymbols.get('*')?.local === '*') { + outgoingDep.usedSymbolsUp.forEach((sResolved, s) => { + if (s === 'default') { + return; + } - let reexported = assetSymbolsInverse?.get(local); - if (reexported != null) { - reexported.forEach(s => { - // see same code above + // 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)) { if (!assetNode.usedSymbols.has('*')) { logFallbackNamespaceInsertion( @@ -553,135 +645,171 @@ export class AssetGraphBuilder { } }); } - } - } - let errors: Array = []; + for (let [s, sResolved] of outgoingDep.usedSymbolsUp) { + if (!outgoingDep.usedSymbolsDown.has(s)) { + // usedSymbolsDown is a superset of usedSymbolsUp + continue; + } - function usedSymbolsUpAmbiguous(old, current, s, value) { - if (old.has(s)) { - let valueOld = old.get(s); - if ( - valueOld !== value && - !( - valueOld?.asset === value.asset && - valueOld?.symbol === value.symbol - ) - ) { - // The dependency points to multiple assets (via an asset group). - current.set(s, undefined); - return; + let local = outgoingDepSymbols.get(s)?.local; + + if (local == null) { + // Caused by '*' => '*', already handled + continue; + } + + let reexported = assetSymbolsInverse?.get(local); + if (reexported != null) { + reexported.forEach(s => { + // see same code above + if (reexportedSymbols.has(s)) { + if (!assetNode.usedSymbols.has('*')) { + logFallbackNamespaceInsertion( + assetNode, + s, + nullthrows(reexportedSymbolsSource.get(s)), + outgoingDep, + ); + } + assetNode.usedSymbols.add('*'); + reexportedSymbols.set(s, {asset: assetNode.id, symbol: s}); + } else { + reexportedSymbols.set(s, sResolved); + reexportedSymbolsSource.set(s, outgoingDep); + } + }); + } } } - current.set(s, value); - } - for (let incomingDep of incomingDeps) { - let incomingDepUsedSymbolsUpOld = incomingDep.usedSymbolsUp; - incomingDep.usedSymbolsUp = new Map(); - let incomingDepSymbols = incomingDep.value.symbols; - if (!incomingDepSymbols) continue; + let errors: Array = []; - let hasNamespaceReexport = incomingDepSymbols.get('*')?.local === '*'; - for (let s of incomingDep.usedSymbolsDown) { - if ( - assetSymbols == null || // Assume everything could be provided if symbols are cleared - assetNode.value.bundleBehavior === BundleBehavior.isolated || - assetNode.value.bundleBehavior === BundleBehavior.inline || - s === '*' || - assetNode.usedSymbols.has(s) - ) { - usedSymbolsUpAmbiguous( - incomingDepUsedSymbolsUpOld, - incomingDep.usedSymbolsUp, - s, - { - asset: assetNode.id, - symbol: s, - }, - ); - } else if (reexportedSymbols.has(s)) { - let reexport = reexportedSymbols.get(s); - let v = - // Forward a reexport only if the current asset is side-effect free and not external - !assetNode.value.sideEffects && reexport != null - ? reexport - : { - asset: assetNode.id, - symbol: s, - }; - usedSymbolsUpAmbiguous( - incomingDepUsedSymbolsUpOld, - incomingDep.usedSymbolsUp, - s, - v, - ); - } else if (!hasNamespaceReexport) { - let loc = incomingDep.value.symbols?.get(s)?.loc; - let [resolutionNodeId] = this.assetGraph.getNodeIdsConnectedFrom( - this.assetGraph.getNodeIdByContentKey(incomingDep.id), - ); - let resolution = nullthrows( - this.assetGraph.getNode(resolutionNodeId), - ); - invariant(resolution && resolution.type === 'asset_group'); - - errors.push({ - message: md`${fromProjectPathRelative( - resolution.value.filePath, - )} does not export '${s}'`, - origin: '@parcel/core', - codeFrames: loc - ? [ - { - filePath: - fromProjectPath( - this.options.projectRoot, - loc?.filePath, - ) ?? undefined, - language: incomingDep.value.sourceAssetType ?? undefined, - codeHighlights: [ - { - start: loc.start, - end: loc.end, - }, - ], - }, - ] - : undefined, - }); + function usedSymbolsUpAmbiguous(old, current, s, value) { + if (old.has(s)) { + let valueOld = old.get(s); + if ( + valueOld !== value && + !( + valueOld?.asset === value.asset && + valueOld?.symbol === value.symbol + ) + ) { + // The dependency points to multiple assets (via an asset group). + current.set(s, undefined); + return; + } } + current.set(s, value); } - if (!equalMap(incomingDepUsedSymbolsUpOld, incomingDep.usedSymbolsUp)) { - changedDeps.add(incomingDep); - incomingDep.usedSymbolsUpDirtyUp = true; - } + for (let incomingDep of incomingDeps) { + let incomingDepUsedSymbolsUpOld = incomingDep.usedSymbolsUp; + incomingDep.usedSymbolsUp = new Map(); + let incomingDepSymbols = incomingDep.value.symbols; + if (!incomingDepSymbols) continue; - incomingDep.excluded = false; - if ( - incomingDep.value.symbols != null && - incomingDep.usedSymbolsUp.size === 0 - ) { - let assetGroups = this.assetGraph.getNodeIdsConnectedFrom( - this.assetGraph.getNodeIdByContentKey(incomingDep.id), - ); - if (assetGroups.length === 1) { - let [assetGroupId] = assetGroups; - let assetGroup = nullthrows(this.assetGraph.getNode(assetGroupId)); + let hasNamespaceReexport = incomingDepSymbols.get('*')?.local === '*'; + for (let s of incomingDep.usedSymbolsDown) { if ( - assetGroup.type === 'asset_group' && - assetGroup.value.sideEffects === false + assetSymbols == null || // Assume everything could be provided if symbols are cleared + assetNode.value.bundleBehavior === BundleBehavior.isolated || + assetNode.value.bundleBehavior === BundleBehavior.inline || + s === '*' || + assetNode.usedSymbols.has(s) ) { - incomingDep.excluded = true; + usedSymbolsUpAmbiguous( + incomingDepUsedSymbolsUpOld, + incomingDep.usedSymbolsUp, + s, + { + asset: assetNode.id, + symbol: s, + }, + ); + } else if (reexportedSymbols.has(s)) { + let reexport = reexportedSymbols.get(s); + let v = + // Forward a reexport only if the current asset is side-effect free and not external + !assetNode.value.sideEffects && reexport != null + ? reexport + : { + asset: assetNode.id, + symbol: s, + }; + usedSymbolsUpAmbiguous( + incomingDepUsedSymbolsUpOld, + incomingDep.usedSymbolsUp, + s, + v, + ); + } else if (!hasNamespaceReexport) { + let loc = incomingDep.value.symbols?.get(s)?.loc; + let [resolutionNodeId] = assetGraph.getNodeIdsConnectedFrom( + assetGraph.getNodeIdByContentKey(incomingDep.id), + ); + let resolution = nullthrows(assetGraph.getNode(resolutionNodeId)); + invariant(resolution && resolution.type === 'asset_group'); + + errors.push({ + message: md`${fromProjectPathRelative( + resolution.value.filePath, + )} does not export '${s}'`, + origin: '@parcel/core', + codeFrames: loc + ? [ + { + filePath: + fromProjectPath(options.projectRoot, loc?.filePath) ?? + undefined, + language: + incomingDep.value.sourceAssetType ?? undefined, + codeHighlights: [ + { + start: loc.start, + end: loc.end, + }, + ], + }, + ] + : undefined, + }); + } + } + + if ( + !equalMap(incomingDepUsedSymbolsUpOld, incomingDep.usedSymbolsUp) + ) { + changedDeps.add(incomingDep); + incomingDep.usedSymbolsUpDirtyUp = true; + } + + incomingDep.excluded = false; + if ( + incomingDep.value.symbols != null && + incomingDep.usedSymbolsUp.size === 0 + ) { + let assetGroups = assetGraph.getNodeIdsConnectedFrom( + assetGraph.getNodeIdByContentKey(incomingDep.id), + ); + if (assetGroups.length === 1) { + let [assetGroupId] = assetGroups; + let assetGroup = nullthrows(assetGraph.getNode(assetGroupId)); + if ( + assetGroup.type === 'asset_group' && + assetGroup.value.sideEffects === false + ) { + incomingDep.excluded = true; + } + } else { + invariant(assetGroups.length === 0); } - } else { - invariant(assetGroups.length === 0); } } - } - return errors; - }); + return errors; + }, + ); + // Sort usedSymbolsUp so they are a consistent order across builds. // This ensures a consistent ordering of these symbols when packaging. // See https://github.com/parcel-bundler/parcel/pull/8212 @@ -690,28 +818,62 @@ export class AssetGraphBuilder { [...dep.usedSymbolsUp].sort(([a], [b]) => a.localeCompare(b)), ); } + + return errors; } propagateSymbolsDown( + assetGraph: AssetGraph, + changedAssets: Set, + assetGroupsWithRemovedParents: Set, visit: ( assetNode: AssetNode, incoming: $ReadOnlyArray, outgoing: $ReadOnlyArray, ) => void, ) { - let rootNodeId = nullthrows( - this.assetGraph.rootNodeId, - 'A root node is required to traverse', - ); - let queue: Set = new Set([rootNodeId]); - let visited = new Set(); + if (changedAssets.size === 0 && assetGroupsWithRemovedParents.size === 0) { + return; + } + + // We care about changed assets and their changed dependencies. So start with the first changed + // asset or dependency and continue while the symbols change. If the queue becomes empty, + // continue with the next unvisited changed asset. + // + // In the end, nodes, which are neither listed in changedAssets nor in + // assetGroupsWithRemovedParents nor reached via a dirty flag, don't have to be visited at all. + // + // In the worst case, some nodes have to be revisited because we don't want to sort the assets + // into topological order. For example in a diamond graph where the join point is visited twice + // via each parent (the numbers signifiying the order of re/visiting, `...` being unvisited). + // However, this only continues as long as there are changes in the used symbols that influence + // child nodes. + // + // | + // ... + // / \ + // 1 4 + // \ / + // 2+5 + // | + // 3+6 + // | + // ... + // | + // + + let unreachedAssets = new Set([ + ...changedAssets, + ...assetGroupsWithRemovedParents, + ]); + let queue = new Set([setPop(unreachedAssets)]); while (queue.size > 0) { - let queuedNodeId = nullthrows(queue.values().next().value); - queue.delete(queuedNodeId); + let queuedNodeId = setPop(queue); + unreachedAssets.delete(queuedNodeId); - let outgoing = this.assetGraph.getNodeIdsConnectedFrom(queuedNodeId); - let node = nullthrows(this.assetGraph.getNode(queuedNodeId)); + let outgoing = assetGraph.getNodeIdsConnectedFrom(queuedNodeId); + let node = nullthrows(assetGraph.getNode(queuedNodeId)); let wasNodeDirty = false; if (node.type === 'dependency' || node.type === 'asset_group') { @@ -720,13 +882,13 @@ export class AssetGraphBuilder { } else if (node.type === 'asset' && node.usedSymbolsDownDirty) { visit( node, - this.assetGraph.getIncomingDependencies(node.value).map(d => { - let dep = this.assetGraph.getNodeByContentKey(d.id); + assetGraph.getIncomingDependencies(node.value).map(d => { + let dep = assetGraph.getNodeByContentKey(d.id); invariant(dep && dep.type === 'dependency'); return dep; }), outgoing.map(dep => { - let depNode = nullthrows(this.assetGraph.getNode(dep)); + let depNode = nullthrows(assetGraph.getNode(dep)); invariant(depNode.type === 'dependency'); return depNode; }), @@ -734,9 +896,8 @@ export class AssetGraphBuilder { node.usedSymbolsDownDirty = false; } - visited.add(queuedNodeId); for (let child of outgoing) { - let childNode = nullthrows(this.assetGraph.getNode(child)); + let childNode = nullthrows(assetGraph.getNode(child)); let childDirty = false; if ( (childNode.type === 'asset' || childNode.type === 'asset_group') && @@ -747,110 +908,150 @@ export class AssetGraphBuilder { } else if (childNode.type === 'dependency') { childDirty = childNode.usedSymbolsDownDirty; } - if (!visited.has(child) || childDirty) { + if (childDirty) { queue.add(child); } } + + if (queue.size === 0 && unreachedAssets.size > 0) { + queue.add(setPop(unreachedAssets)); + } } } propagateSymbolsUp( + assetGraph: AssetGraph, + changedAssets: Set, + changedDepsUsedSymbolsUpDirtyDown: Set, + previousErrors: ?Map>, visit: ( assetNode: AssetNode, incoming: $ReadOnlyArray, outgoing: $ReadOnlyArray, ) => Array, - ): void { - let rootNodeId = nullthrows( - this.assetGraph.rootNodeId, - 'A root node is required to traverse', - ); - - let errors = new Map>(); - - let dirtyDeps = new Set(); - let visited = new Set([rootNodeId]); - // post-order dfs - const walk = (nodeId: NodeId) => { - let node = nullthrows(this.assetGraph.getNode(nodeId)); - let outgoing = this.assetGraph.getNodeIdsConnectedFrom(nodeId); - for (let childId of outgoing) { - if (!visited.has(childId)) { - visited.add(childId); - walk(childId); - let child = nullthrows(this.assetGraph.getNode(childId)); - if (node.type === 'asset') { - invariant(child.type === 'dependency'); - if (child.usedSymbolsUpDirtyUp) { - node.usedSymbolsUpDirty = true; - child.usedSymbolsUpDirtyUp = false; + ): Map> { + // For graphs in general (so with cyclic dependencies), some nodes will have to be revisited. So + // run a regular queue-based BFS for anything that's still dirty. + // + // (Previously, there was first a recursive post-order DFS, with the idea that all children of a + // node should be processed first. With a tree, this would result in a minimal amount of work by + // processing every asset exactly once and then the remaining cycles would have been handled + // with the loop. This was slightly faster for initial builds but had O(project) instead of + // O(changes).) + + let errors: Map> = previousErrors + ? // Some nodes might have been removed since the last build + new Map([...previousErrors].filter(([n]) => assetGraph.hasNode(n))) + : new Map(); + + let changedDepsUsedSymbolsUpDirtyDownAssets = new Set([ + ...[...changedDepsUsedSymbolsUpDirtyDown] + .reverse() + .flatMap(id => getDependencyResolution(assetGraph, id)), + ...changedAssets, + ]); + + // Do a more efficient full traversal (less recomputations) if more than half of the assets + // changed. + let runFullPass = + // If there are n nodes in the graph, then the asset count is approximately + // n/6 (for every asset, there are ~4 dependencies and ~1 asset_group). + assetGraph.nodes.size * (1 / 6) * 0.5 < + changedDepsUsedSymbolsUpDirtyDownAssets.size; + + let dirtyDeps; + if (runFullPass) { + dirtyDeps = new Set(); + let rootNodeId = nullthrows( + assetGraph.rootNodeId, + 'A root node is required to traverse', + ); + let visited = new Set([rootNodeId]); + const walk = (nodeId: NodeId) => { + let node = nullthrows(assetGraph.getNode(nodeId)); + let outgoing = assetGraph.getNodeIdsConnectedFrom(nodeId); + for (let childId of outgoing) { + if (!visited.has(childId)) { + visited.add(childId); + walk(childId); + let child = nullthrows(assetGraph.getNode(childId)); + if (node.type === 'asset') { + invariant(child.type === 'dependency'); + if (child.usedSymbolsUpDirtyUp) { + node.usedSymbolsUpDirty = true; + child.usedSymbolsUpDirtyUp = false; + } } } } - } - if (node.type === 'asset') { - let incoming = this.assetGraph - .getIncomingDependencies(node.value) - .map(d => { - let n = this.assetGraph.getNodeByContentKey(d.id); - invariant(n && n.type === 'dependency'); - return n; - }); - for (let dep of incoming) { - if (dep.usedSymbolsUpDirtyDown) { - dep.usedSymbolsUpDirtyDown = false; - node.usedSymbolsUpDirty = true; + if (node.type === 'asset') { + let incoming = assetGraph + .getIncomingDependencies(node.value) + .map(d => { + let n = assetGraph.getNodeByContentKey(d.id); + invariant(n && n.type === 'dependency'); + return n; + }); + for (let dep of incoming) { + if (dep.usedSymbolsUpDirtyDown) { + dep.usedSymbolsUpDirtyDown = false; + node.usedSymbolsUpDirty = true; + } } - } - if (node.usedSymbolsUpDirty) { - let e = visit( - node, - incoming, - outgoing.map(depNodeId => { - let depNode = nullthrows(this.assetGraph.getNode(depNodeId)); - invariant(depNode.type === 'dependency'); - return depNode; - }), - ); - if (e.length > 0) { - node.usedSymbolsUpDirty = true; - errors.set(nodeId, e); - } else { - node.usedSymbolsUpDirty = false; - errors.delete(nodeId); + if (node.usedSymbolsUpDirty) { + let e = visit( + node, + incoming, + outgoing.map(depNodeId => { + let depNode = nullthrows(assetGraph.getNode(depNodeId)); + invariant(depNode.type === 'dependency'); + return depNode; + }), + ); + if (e.length > 0) { + node.usedSymbolsUpDirty = true; + errors.set(nodeId, e); + } else { + node.usedSymbolsUpDirty = false; + errors.delete(nodeId); + } } - } - } else if (node.type === 'dependency') { - if (node.usedSymbolsUpDirtyUp) { - dirtyDeps.add(nodeId); } else { - dirtyDeps.delete(nodeId); + if (node.type === 'dependency') { + if (node.usedSymbolsUpDirtyUp) { + dirtyDeps.add(nodeId); + } else { + dirtyDeps.delete(nodeId); + } + } } - } - }; - walk(rootNodeId); - // traverse circular dependencies if necessary (ancestors of `dirtyDeps`) - visited = new Set(); - let queue = new Set(dirtyDeps); + }; + walk(rootNodeId); + } + + let queue = dirtyDeps ?? changedDepsUsedSymbolsUpDirtyDownAssets; while (queue.size > 0) { - let queuedNodeId = nullthrows(queue.values().next().value); - queue.delete(queuedNodeId); - visited.add(queuedNodeId); - let node = nullthrows(this.assetGraph.getNode(queuedNodeId)); + let queuedNodeId = setPop(queue); + let node = nullthrows(assetGraph.getNode(queuedNodeId)); if (node.type === 'asset') { - let incoming = this.assetGraph + let incoming = assetGraph .getIncomingDependencies(node.value) .map(dep => { - let depNode = this.assetGraph.getNodeByContentKey(dep.id); + let depNode = assetGraph.getNodeByContentKey(dep.id); invariant(depNode && depNode.type === 'dependency'); return depNode; }); - let outgoing = this.assetGraph + for (let dep of incoming) { + if (dep.usedSymbolsUpDirtyDown) { + dep.usedSymbolsUpDirtyDown = false; + node.usedSymbolsUpDirty = true; + } + } + let outgoing = assetGraph .getNodeIdsConnectedFrom(queuedNodeId) .map(depNodeId => { - let depNode = nullthrows(this.assetGraph.getNode(depNodeId)); - + let depNode = nullthrows(assetGraph.getNode(depNodeId)); invariant(depNode.type === 'dependency'); return depNode; }); @@ -860,6 +1061,7 @@ export class AssetGraphBuilder { dep.usedSymbolsUpDirtyUp = false; } } + if (node.usedSymbolsUpDirty) { let e = visit(node, incoming, outgoing); if (e.length > 0) { @@ -870,26 +1072,21 @@ export class AssetGraphBuilder { errors.delete(queuedNodeId); } } + for (let i of incoming) { if (i.usedSymbolsUpDirtyUp) { - queue.add(this.assetGraph.getNodeIdByContentKey(i.id)); + queue.add(assetGraph.getNodeIdByContentKey(i.id)); } } } else { - let connectedNodes = - this.assetGraph.getNodeIdsConnectedTo(queuedNodeId); + let connectedNodes = assetGraph.getNodeIdsConnectedTo(queuedNodeId); if (connectedNodes.length > 0) { queue.add(...connectedNodes); } } } - // Just throw the first error. Since errors can bubble (e.g. reexporting a reexported symbol also fails), - // determining which failing export is the root cause is nontrivial (because of circular dependencies). - if (errors.size > 0) { - throw new ThrowableDiagnostic({ - diagnostic: [...errors.values()][0], - }); - } + + return errors; } shouldSkipRequest(nodeId: NodeId): boolean { @@ -1007,6 +1204,7 @@ export class AssetGraphBuilder { } } this.changedAssets.set(asset.id, asset); + this.changedAssetsPropagation.add(asset.id); } this.assetGraph.resolveAssetGroup(input, assets, request.id); } else { @@ -1042,6 +1240,29 @@ export class AssetGraphBuilder { } } +function equalSet(a: $ReadOnlySet, b: $ReadOnlySet) { + return a.size === b.size && [...a].every(i => b.has(i)); +} + +function getDependencyResolution( + graph: AssetGraph, + depId: ContentKey, +): Array { + let depNodeId = graph.getNodeIdByContentKey(depId); + let connected = graph.getNodeIdsConnectedFrom(depNodeId); + invariant(connected.length <= 1); + let child = connected[0]; + if (child) { + let childNode = nullthrows(graph.getNode(child)); + if (childNode.type === 'asset_group') { + return graph.getNodeIdsConnectedFrom(child); + } else { + return [child]; + } + } + return []; +} + function equalMap( a: $ReadOnlyMap, b: $ReadOnlyMap, @@ -1055,6 +1276,8 @@ function equalMap( return true; } -function equalSet(a: $ReadOnlySet, b: $ReadOnlySet) { - return a.size === b.size && [...a].every(i => b.has(i)); +function setPop(set: Set): T { + let v = nullthrows(set.values().next().value); + set.delete(v); + return v; } diff --git a/packages/core/core/src/requests/BundleGraphRequest.js b/packages/core/core/src/requests/BundleGraphRequest.js index a5ab0daefcc..45feda04481 100644 --- a/packages/core/core/src/requests/BundleGraphRequest.js +++ b/packages/core/core/src/requests/BundleGraphRequest.js @@ -272,11 +272,19 @@ class BundlerRunner { try { if (previousBundleGraphResult) { internalBundleGraph = previousBundleGraphResult.bundleGraph; - for (let changedAsset of changedAssets.values()) { - internalBundleGraph.updateAsset(changedAsset); + for (let changedAssetId of changedAssets.keys()) { + // Copy over the whole node to also have correct symbol data + let changedAssetNode = nullthrows( + graph.getNodeByContentKey(changedAssetId), + ); + invariant(changedAssetNode.type === 'asset'); + internalBundleGraph.updateAsset(changedAssetNode); } } else { - internalBundleGraph = InternalBundleGraph.fromAssetGraph(graph); + internalBundleGraph = InternalBundleGraph.fromAssetGraph( + graph, + this.options.mode === 'production', + ); invariant(internalBundleGraph != null); // ensures the graph was created await dumpGraphToGraphViz( diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index ad4636dee47..275a7705cfd 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -331,12 +331,21 @@ export type DependencyNode = {| Symbol, {|asset: ContentKey, symbol: ?Symbol|} | void | null, >, - /** for the "down" pass, the dependency resolution asset needs to be updated */ + /* + * For the "down" pass, the resolutionAsset needs to be updated. + * This is set when the AssetGraphBuilder adds/removes/updates nodes. + */ usedSymbolsDownDirty: boolean, - /** for the "up" pass, the parent asset needs to be updated */ - usedSymbolsUpDirtyUp: boolean, - /** for the "up" pass, the dependency resolution asset needs to be updated */ + /** + * In the down pass, `usedSymbolsDown` changed. This needs to be propagated to the resolutionAsset + * in the up pass. + */ usedSymbolsUpDirtyDown: boolean, + /** + * In the up pass, `usedSymbolsUp` changed. This needs to be propagated to the sourceAsset in the + * up pass. + */ + usedSymbolsUpDirtyUp: boolean, /** dependency was excluded (= no used symbols (globally) & side-effect free) */ excluded: boolean, |}; diff --git a/packages/core/core/test/BundleGraph.test.js b/packages/core/core/test/BundleGraph.test.js index 9d65a8e4948..83505d522ec 100644 --- a/packages/core/core/test/BundleGraph.test.js +++ b/packages/core/core/test/BundleGraph.test.js @@ -23,6 +23,7 @@ describe('BundleGraph', () => { it('assigns publicIds to assets', () => { let bundleGraph = BundleGraph.fromAssetGraph( createMockAssetGraph([id1, id2]), + false, ); assert.deepEqual( getAssets(bundleGraph).map(a => bundleGraph.getAssetPublicId(a)), @@ -33,6 +34,7 @@ describe('BundleGraph', () => { it('uses a longer publicId if there is a collision', () => { let bundleGraph = BundleGraph.fromAssetGraph( createMockAssetGraph([id1, id1.slice(0, 16) + '7' + id1.slice(17)]), + false, ); assert.deepEqual( getAssets(bundleGraph).map(a => bundleGraph.getAssetPublicId(a)), diff --git a/packages/core/core/test/PublicBundle.test.js b/packages/core/core/test/PublicBundle.test.js index 97338f16cd0..39fe7baf6ba 100644 --- a/packages/core/core/test/PublicBundle.test.js +++ b/packages/core/core/test/PublicBundle.test.js @@ -42,7 +42,6 @@ describe('Public Bundle', () => { assetPublicIds: new Set(), publicIdByAssetId: new Map(), bundleContentHashes: new Map(), - symbolPropagationRan: false, }); }); diff --git a/packages/core/core/test/PublicMutableBundleGraph.test.js b/packages/core/core/test/PublicMutableBundleGraph.test.js index 20fe2fae955..daf96fcc0a9 100644 --- a/packages/core/core/test/PublicMutableBundleGraph.test.js +++ b/packages/core/core/test/PublicMutableBundleGraph.test.js @@ -28,6 +28,7 @@ describe('PublicMutableBundleGraph', () => { it('creates publicIds for bundles', () => { let internalBundleGraph = InternalBundleGraph.fromAssetGraph( createMockAssetGraph(), + false, ); let mutableBundleGraph = new MutableBundleGraph( internalBundleGraph, @@ -63,6 +64,7 @@ describe('PublicMutableBundleGraph', () => { it('is safe to add a bundle to a bundleGroup multiple times', () => { let internalBundleGraph = InternalBundleGraph.fromAssetGraph( createMockAssetGraph(), + false, ); let mutableBundleGraph = new MutableBundleGraph( internalBundleGraph, diff --git a/packages/core/core/test/SymbolPropagation.test.js b/packages/core/core/test/SymbolPropagation.test.js new file mode 100644 index 00000000000..8e4a597267f --- /dev/null +++ b/packages/core/core/test/SymbolPropagation.test.js @@ -0,0 +1,716 @@ +// @flow strict-local +import assert from 'assert'; +import invariant from 'assert'; +import nullthrows from 'nullthrows'; +import type {FilePath, SourceLocation, Meta, Symbol} from '@parcel/types'; +import type {ContentKey, NodeId} from '@parcel/graph'; +import type {Diagnostic} from '@parcel/diagnostic'; +import ThrowableDiagnostic from '@parcel/diagnostic'; +import {setEqual} from '@parcel/utils'; +import AssetGraph, { + nodeFromAssetGroup, + nodeFromDep, + nodeFromEntryFile, + nodeFromAsset, +} from '../src/AssetGraph'; +import {createDependency as _createDependency} from '../src/Dependency'; +import {createAsset as _createAsset} from '../src/assetUtils'; +import { + toProjectPath as _toProjectPath, + type ProjectPath, +} from '../src/projectPath'; +import {propagateSymbols} from '../src/SymbolPropagation'; +import dumpGraphToGraphViz from '../src/dumpGraphToGraphViz'; +import {DEFAULT_ENV, DEFAULT_OPTIONS, DEFAULT_TARGETS} from './test-utils'; +import type { + Asset, + AssetNode, + AssetGraphNode, + Dependency, + DependencyNode, +} from '../src/types'; + +const stats = {size: 0, time: 0}; + +function createAsset(opts) { + return _createAsset('/', opts); +} + +function createDependency(opts) { + return _createDependency('/', opts); +} + +function toProjectPath(p) { + return _toProjectPath('/', p); +} + +function fromProjectPathUnix(p: ProjectPath) { + // $FlowFixMe + return '/' + p; +} + +function nullthrowsAssetNode(v: ?AssetGraphNode): AssetNode { + invariant(v?.type === 'asset'); + return v; +} +function nullthrowsDependencyNode(v: ?AssetGraphNode): DependencyNode { + invariant(v?.type === 'dependency'); + return v; +} + +function createAssetGraph( + assets: Array< + [ + FilePath, + /* symbols (or cleared) */ ?Array< + [Symbol, {|local: Symbol, loc?: ?SourceLocation, meta?: ?Meta|}], + >, + /* sideEffects */ boolean, + ], + >, + dependencies: Array< + [ + /* from */ FilePath, + /* to */ FilePath, + /* symbols (or cleared) */ ?Array< + [ + Symbol, + {| + local: Symbol, + loc?: ?SourceLocation, + isWeak: boolean, + meta?: ?Meta, + |}, + ], + >, + ], + >, + isLibrary?: boolean, +) { + let graph = new AssetGraph(); + let entryFilePath = '/index.js'; + graph.setRootConnections({ + entries: [toProjectPath(entryFilePath)], + }); + let entry = { + filePath: toProjectPath(entryFilePath), + packagePath: toProjectPath('/'), + }; + let entryNodeContentKey = nodeFromEntryFile(entry).id; + graph.resolveEntry(toProjectPath(entryFilePath), [entry], '1'); + graph.resolveTargets(entry, DEFAULT_TARGETS, '2'); + let entryDependencyId = graph.getNodeIdsConnectedFrom( + graph.getNodeIdByContentKey(entryNodeContentKey), + )[0]; + if (isLibrary) { + let entryDependencyNode = nullthrows(graph.getNode(entryDependencyId)); + invariant(entryDependencyNode.type === 'dependency'); + entryDependencyNode.value.symbols = new Map([ + ['*', {local: '*', isWeak: true, loc: null}], + ]); + entryDependencyNode.usedSymbolsDown.add('*'); + entryDependencyNode.usedSymbolsUp.set('*', undefined); + } + + let assetId = 1; + let changedAssets = new Map(); + let assetGroupNodes = new Map(); + let assetNodes = new Map(); + for (let [filePath, symbols, sideEffects] of assets) { + let assetGroup = nodeFromAssetGroup({ + filePath: toProjectPath(filePath), + env: DEFAULT_ENV, + sideEffects, + }); + let assetGroupNodeId = graph.addNodeByContentKey(assetGroup.id, assetGroup); + assetGroupNodes.set(filePath, assetGroupNodeId); + + let asset = nodeFromAsset( + createAsset({ + id: String(assetId), + filePath: toProjectPath(filePath), + type: 'js', + isSource: true, + sideEffects, + stats, + symbols: symbols ? new Map(symbols) : symbols, + env: DEFAULT_ENV, + }), + ); + let assetNodeId = graph.addNodeByContentKey(asset.id, asset); + assetNodes.set(filePath, assetNodeId); + changedAssets.set(String(assetId), asset.value); + + graph.addEdge(assetGroupNodeId, assetNodeId); + + assetId++; + } + + for (let [from, to, symbols] of dependencies) { + let dependencyNode = nodeFromDep( + createDependency({ + specifier: to, + specifierType: 'esm', + env: DEFAULT_ENV, + symbols: symbols ? new Map(symbols) : symbols, + sourcePath: from, + sourceAssetId: from, + }), + ); + let dependencyNodeId = graph.addNodeByContentKey( + dependencyNode.id, + dependencyNode, + ); + graph.addEdge(nullthrows(assetNodes.get(from)), dependencyNodeId); + graph.addEdge(dependencyNodeId, nullthrows(assetGroupNodes.get(to))); + } + + let entryAssetGroup = nullthrows(assetGroupNodes.get(entryFilePath)); + graph.addEdge(entryDependencyId, entryAssetGroup); + + return {graph, changedAssets}; +} + +function assertUsedSymbols( + graph: AssetGraph, + _expectedAsset: Array<[FilePath, /* usedSymbols */ Array]>, + _expectedDependency: Array< + [ + FilePath, + FilePath, + /* usedSymbols */ Array<[Symbol, ?[FilePath, ?Symbol]] | [Symbol]> | null, + ], + >, + isLibrary?: boolean, +) { + let expectedAsset = new Map( + _expectedAsset.map(([f, symbols]) => [f, symbols]), + ); + let expectedDependency = new Map( + _expectedDependency.map(([from, to, sym]) => [ + from + ':' + to, + // $FlowFixMe[invalid-tuple-index] + sym ? sym.map(v => [v[0], v[1] ?? [to, v[0]]]) : sym, + ]), + ); + + if (isLibrary) { + let entryDep = nullthrows( + [...graph.nodes.values()].find( + n => n.type === 'dependency' && n.value.sourceAssetId == null, + ), + ); + invariant(entryDep.type === 'dependency'); + assertDependencyUsedSymbols( + entryDep.usedSymbolsUp, + new Map([['*', undefined]]), + 'entryDep', + ); + } + + function assertDependencyUsedSymbols(usedSymbolsUp, expectedMap, id) { + assertSetEqual( + new Set(usedSymbolsUp.keys()), + new Set(expectedMap.keys()), + id, + ); + + for (let [s, resolved] of usedSymbolsUp) { + let exp = expectedMap.get(s); + if (resolved && exp) { + let asset = nullthrows(graph.getNodeByContentKey(resolved.asset)); + invariant(asset.type === 'asset'); + assert.strictEqual( + fromProjectPathUnix(asset.value.filePath), + exp[0], + `dep ${id}@${s} resolved asset: ${fromProjectPathUnix( + asset.value.filePath, + )} !== ${exp[0]}`, + ); + assert.strictEqual( + resolved.symbol, + exp[1], + `dep ${id}@${s} resolved symbol: ${String( + resolved.symbol, + )} !== ${String(exp[1])}`, + ); + } else { + assert.equal(resolved, exp); + } + } + } + + for (let [nodeId, node] of graph.nodes) { + if (node.type === 'asset') { + let filePath = fromProjectPathUnix(node.value.filePath); + let expected = new Set(nullthrows(expectedAsset.get(filePath))); + assertSetEqual(node.usedSymbols, expected, filePath); + } else if (node.type === 'dependency' && node.value.sourcePath != null) { + let resolutionId = graph.getNodeIdsConnectedFrom(nodeId)[0]; + let resolution = nullthrows(graph.getNode(resolutionId)); + invariant(resolution.type === 'asset_group'); + let to = resolution.value.filePath; + + let id = + fromProjectPathUnix(nullthrows(node.value.sourcePath)) + + ':' + + fromProjectPathUnix(nullthrows(to)); + let expected = expectedDependency.get(id); + if (!expected) { + assert(expected === null); + assertSetEqual(new Set(node.usedSymbolsUp.keys()), new Set(), id); + assert(node.excluded, `${id} should be excluded`); + } else { + assert(!node.excluded, `${id} should not be excluded`); + let expectedMap = new Map(expected); + + assertDependencyUsedSymbols(node.usedSymbolsUp, expectedMap, id); + } + } + } +} + +function assertSetEqual( + actual: $ReadOnlySet, + expected: $ReadOnlySet, + prefix?: string = '', +) { + assert( + setEqual(actual, expected), + `${prefix} [${[...actual].join(',')}] wasn't [${[...expected].join(',')}]`, + ); +} + +async function testPropagation( + assets: Array< + [ + FilePath, + /* symbols (or cleared) */ ?Array< + [Symbol, {|local: Symbol, loc?: ?SourceLocation, meta?: ?Meta|}], + >, + /* sideEffects */ boolean, + /* usedSymbols */ Array, + ], + >, + dependencies: Array< + [ + /* from */ FilePath, + /* to */ FilePath, + /* symbols (or cleared) */ ?Array< + [ + Symbol, + {| + local: Symbol, + loc?: ?SourceLocation, + isWeak: boolean, + meta?: ?Meta, + |}, + ], + >, + /* usedSymbols */ Array< + [Symbol, ?[FilePath, ?Symbol]] | [Symbol], + > | /* excluded */ null, + ], + >, + isLibrary?: boolean, +): Promise { + let {graph, changedAssets} = createAssetGraph( + assets.map(([f, symbols, sideEffects]) => [f, symbols, sideEffects]), + dependencies.map(([from, to, symbols]) => [from, to, symbols]), + isLibrary, + ); + await dumpGraphToGraphViz(graph, 'test_before'); + + handlePropagationErrors( + propagateSymbols({ + options: DEFAULT_OPTIONS, + assetGraph: graph, + changedAssetsPropagation: new Set(changedAssets.keys()), + assetGroupsWithRemovedParents: new Set(), + previousErrors: undefined, + }), + ); + + await dumpGraphToGraphViz(graph, 'test_after'); + + assertUsedSymbols( + graph, + assets.map(([f, , , usedSymbols]) => [f, usedSymbols]), + dependencies.map(([from, to, , usedSymbols]) => [from, to, usedSymbols]), + isLibrary, + ); + + return graph; +} + +function handlePropagationErrors(errors: Map>) { + if (errors.size > 0) { + throw new ThrowableDiagnostic({ + diagnostic: [...errors.values()][0], + }); + } +} + +function assertPropagationErrors( + graph: AssetGraph, + actual: Map>, + expected: Iterable<[FilePath, Array]>, +) { + assert.deepEqual( + [...actual].map(([k, v]) => [ + nullthrowsAssetNode(graph.getNode(k)).value.filePath, + v, + ]), + [...expected], + ); +} + +function changeDependency( + graph: AssetGraph, + from: FilePath, + to: FilePath, + cb: ($NonMaybeType) => void, +): Iterable<[ContentKey, Asset]> { + let sourceAssetNode = nullthrowsAssetNode( + [...graph.nodes.values()].find( + n => n.type === 'asset' && n.value.filePath === from, + ), + ); + sourceAssetNode.usedSymbolsDownDirty = true; + let depNode = nullthrowsDependencyNode( + [...graph.nodes.values()].find( + n => + n.type === 'dependency' && + n.value.sourcePath === from && + n.value.specifier === to, + ), + ); + cb(nullthrows(depNode.value.symbols)); + return [[sourceAssetNode.id, sourceAssetNode.value]]; +} + +function changeAsset( + graph: AssetGraph, + asset: FilePath, + cb: ($NonMaybeType) => void, +): Iterable<[ContentKey, Asset]> { + let node = nullthrowsAssetNode( + [...graph.nodes.values()].find( + n => n.type === 'asset' && n.value.filePath === asset, + ), + ); + node.usedSymbolsUpDirty = true; + node.usedSymbolsDownDirty = true; + cb(nullthrows(node.value.symbols)); + return [[node.id, node.value]]; +} + +// process.env.PARCEL_DUMP_GRAPHVIZ = ''; +// process.env.PARCEL_DUMP_GRAPHVIZ = 'symbols'; + +describe('SymbolPropagation', () => { + it('basic tree', async () => { + // prettier-ignore + await testPropagation( + [ + ['/index.js', [], true, []], + ['/lib.js', [['f', {local: 'lib1$foo'}], ['b', {local: 'lib2$bar'}]], false, []], + ['/lib1.js', [['foo', {local: 'v'}]], false, ['foo']], + ['/lib2.js', [['bar', {local: 'v'}]], false, []], + ], + [ + ['/index.js', '/lib.js', [['f', {local: 'f', isWeak: false}]], [['f', ['/lib1.js', 'foo']]]], + ['/lib.js', '/lib1.js', [['foo', {local: 'lib1$foo', isWeak: true}]], [['foo']]], + ['/lib.js', '/lib2.js', [['bar', {local: 'lib2$bar', isWeak: true}]], null], + ], + ); + }); + + it('basic tree - dependency symbol change export', async () => { + // prettier-ignore + let graph = await testPropagation( + [ + ['/index.js', [], true, []], + ['/lib.js', [['f', {local: 'f'}], ['b', {local: 'b'}]], true, ['f']], + ], + [ + ['/index.js', '/lib.js', [['f', {local: 'f', isWeak: false}]], [['f']]], + ], + ); + + let changedAssets = [ + ...changeDependency(graph, 'index.js', '/lib.js', symbols => { + symbols.set('b', { + local: 'b', + isWeak: false, + loc: undefined, + }); + }), + ]; + propagateSymbols({ + options: DEFAULT_OPTIONS, + assetGraph: graph, + changedAssetsPropagation: new Set(new Map(changedAssets).keys()), + assetGroupsWithRemovedParents: new Set(), + }); + + // prettier-ignore + assertUsedSymbols(graph, + [ + ['/index.js', []], + ['/lib.js', ['f', 'b']], + ], + [ + ['/index.js', '/lib.js', [['f'], ['b']]], + ], + ); + }); + + it('basic tree - dependency symbol change import and error', async () => { + // prettier-ignore + let graph = await testPropagation( + [ + ['/index.js', [], true, []], + ['/lib.js', [['f', {local: 'f'}]], true, ['f']], + ], + [ + ['/index.js', '/lib.js', [['f', {local: 'f', isWeak: false}]], [['f']]], + ], + ); + + let changedAssets = [ + ...changeDependency(graph, 'index.js', '/lib.js', symbols => { + symbols.delete('f'); + symbols.set('f2', { + local: 'f2', + isWeak: false, + loc: undefined, + }); + }), + ]; + let errors = propagateSymbols({ + options: DEFAULT_OPTIONS, + assetGraph: graph, + changedAssetsPropagation: new Set(new Map(changedAssets).keys()), + assetGroupsWithRemovedParents: new Set(), + }); + + assertPropagationErrors(graph, errors, [ + [ + 'lib.js', + [ + { + message: "lib.js does not export 'f2'", + origin: '@parcel/core', + codeFrames: undefined, + }, + ], + ], + ]); + }); + + it('basic tree - asset symbol change export and error', async () => { + // prettier-ignore + let graph = await testPropagation( + [ + ['/index.js', [], true, []], + ['/lib.js', [['f', {local: 'f'}]], true, ['f']], + ], + [ + ['/index.js', '/lib.js', [['f', {local: 'f', isWeak: false}]], [['f']]], + ], + ); + + let changedAssets = [ + ...changeAsset(graph, 'lib.js', symbols => { + symbols.delete('f'); + symbols.set('f2', { + local: 'f2', + loc: undefined, + }); + }), + ]; + + let errors = propagateSymbols({ + options: DEFAULT_OPTIONS, + assetGraph: graph, + changedAssetsPropagation: new Set(new Map(changedAssets).keys()), + assetGroupsWithRemovedParents: new Set(), + }); + + assertPropagationErrors(graph, errors, [ + [ + 'lib.js', + [ + { + message: "lib.js does not export 'f'", + origin: '@parcel/core', + codeFrames: undefined, + }, + ], + ], + ]); + }); + + it('basic tree - dependency symbol change reexport', async () => { + // prettier-ignore + let graph = await testPropagation( + [ + ['/index.js', [], true, []], + ['/lib.js', [['f', {local: 'lib1$foo'}], ['b', {local: 'lib2$bar'}]], true, []], + ['/lib1.js', [['foo', {local: 'v'}]], true, ['foo']], + ['/lib2.js', [['bar', {local: 'v'}]], true, []], + ], + [ + ['/index.js', '/lib.js', [['f', {local: 'f', isWeak: false}]], [['f']]], + ['/lib.js', '/lib1.js', [['foo', {local: 'lib1$foo', isWeak: true}]], [['foo']]], + ['/lib.js', '/lib2.js', [['bar', {local: 'lib2$bar', isWeak: true}]], []], + ], + ); + + let changedAssets = [ + ...changeDependency(graph, 'index.js', '/lib.js', symbols => { + symbols.set('b', { + local: 'b', + isWeak: false, + loc: undefined, + }); + }), + ]; + propagateSymbols({ + options: DEFAULT_OPTIONS, + assetGraph: graph, + changedAssetsPropagation: new Set(new Map(changedAssets).keys()), + assetGroupsWithRemovedParents: new Set(), + }); + + // prettier-ignore + assertUsedSymbols(graph, + [ + ['/index.js', []], + ['/lib.js', []], + ['/lib1.js', ['foo']], + ['/lib2.js', ['bar']], + ], + [ + ['/index.js', '/lib.js', [['f'],['b']]], + ['/lib.js', '/lib1.js', [['foo']]], + ['/lib.js', '/lib2.js', [['bar']]], + ], + ); + }); + + it('basic tree with reexport-all', async () => { + // prettier-ignore + await testPropagation( + [ + ['/index.js', [], true, []], + ['/lib.js', [], false, []], + ['/lib1.js', [['foo', {local: 'v'}]], false, ['foo']], + ['/lib2.js', [['bar', {local: 'v'}]], false, []], + ], + [ + ['/index.js', '/lib.js', [['foo', {local: 'foo', isWeak: false}]], [['foo', ['/lib1.js', 'foo']]]], + ['/lib.js', '/lib1.js', [['*', {local: '*', isWeak: true}]], [['foo']]], + ['/lib.js', '/lib2.js', [['*', {local: '*', isWeak: true}]], null], + ], + ); + }); + + it('dependency with * imports everything', async () => { + // prettier-ignore + await testPropagation( + [ + ['/index.js', [], true, []], + ['/lib.js', [['a', {local: 'lib1$foo'}], ['b', {local: 'lib1$b'}]], true, ['*']], + ['/lib1.js', [['b', {local: 'v'}]], true, ['b']], + ['/lib2.js', [['c', {local: 'v'}]], true, ['*']], + ], + [ + ['/index.js', '/lib.js', [['*', {local: 'lib', isWeak: false}]], [['*']]], + ['/lib.js', '/lib1.js', [['b', {local: 'lib1$foo', isWeak: true}]], [['b']]], + // TODO should usedSymbolsUp actually list the individual symbols instead of '*'? + ['/lib.js', '/lib2.js', [['*', {local: '*', isWeak: true}]], [['*']]], + ], + ); + }); + + it('dependency with cleared symbols imports side-effect-full parts', async () => { + // prettier-ignore + await testPropagation( + [ + ['/index.js', [], true, []], + ['/lib.js', [['a', {local: 'lib1$foo'}], ['b', {local: 'lib$b'}]], true, ['b']], + ['/lib1.js', [['b', {local: 'v'}]], true, ['b']], + ['/lib2.js', [['c', {local: 'v'}]], false, ['*']], + ], + [ + ['/index.js', '/lib.js', null, []], + ['/lib.js', '/lib1.js', [['b', {local: 'lib1$foo', isWeak: true}]], [['b']]], + ['/lib.js', '/lib2.js', [['*', {local: '*', isWeak: true}]], [['*']]], + ], + ); + }); + + it('dependency with cleared symbols imports side-effect-free package', async () => { + // prettier-ignore + await testPropagation( + [ + ['/index.js', [], true, []], + ['/lib.js', [['a', {local: 'lib$a'}], ['b', {local: 'lib1$b'}], ['c', {local: 'lib2$c'}]], false, ['a']], + ['/lib1.js', [['b', {local: 'v'}]], false, ['b']], + ['/lib2.js', [['c', {local: 'v'}]], false, ['c']], + ['/lib3.js', [['d', {local: 'v'}]], false, ['*']], + ], + [ + ['/index.js', '/lib.js', null, []], + ['/lib.js', '/lib1.js', [['b', {local: 'lib1$b', isWeak: true}]], [['b']]], + ['/lib.js', '/lib2.js', [['c', {local: 'lib2$c', isWeak: true}]], [['c']]], + ['/lib.js', '/lib3.js', [['*', {local: '*', isWeak: true}]], [['*']]], + ], + ); + }); + + it('library build with entry dependency', async () => { + // prettier-ignore + await testPropagation( + [ + ['/index.js', [["foo", {local: "foo"}], ['b', {local: 'b$b'}]], true, ['*']], + ], + [], + true + ); + }); + + it('library build with entry dependency and reexport', async () => { + // prettier-ignore + await testPropagation( + [ + ['/index.js', [["foo", {local: "foo"}], ['b', {local: 'b$b'}]], true, ['*']], + ['/b.js', [['b', {local: 'b'}]], true, ['b']], + ], + [ + ['/index.js', '/b.js', [['b', {local: 'b$b', isWeak: false}]], [['b']]], + ], + true + ); + }); + + it('cyclic dependency', async () => { + // prettier-ignore + await testPropagation( + [ + ['/index.js', [], true, []], + ['/a.js', [['a', {local: 'b$b'}], ['real', {local: 'real'}]], true, ['real']], + ['/b.js', [['b', {local: 'c$c'}]], true, []], + ['/c.js', [['c', {local: 'a$real'}]], true, []], + ], + [ + ['/index.js', '/a.js', [['a', {local: 'a', isWeak: false}]], [['a']]], + ['/a.js', '/b.js', [['b', {local: 'b$b', isWeak: true}]], [['b']]], + ['/b.js', '/c.js', [['c', {local: 'c$c', isWeak: true}]], [['c']]], + ['/c.js', '/a.js', [['real', {local: 'a$real', isWeak: true}]], [['real']]], + ], + ); + }); +}); diff --git a/packages/core/integration-tests/test/incremental-bundling.js b/packages/core/integration-tests/test/incremental-bundling.js index 13b980f34e7..4eacb8f4c36 100644 --- a/packages/core/integration-tests/test/incremental-bundling.js +++ b/packages/core/integration-tests/test/incremental-bundling.js @@ -483,31 +483,28 @@ console.log(a);`, await overlayFS.mkdirp(fixture); subscription = await b.watch(); - let event = await getNextBuildSuccess(b); + await getNextBuildSuccess(b); assertTimesBundled(defaultBundlerSpy.callCount, 1); await overlayFS.writeFile( path.join(fixture, 'index.js'), - `import a from './a'; -import b from './b'; + `import {a} from './a'; +import {b} from './b'; console.log('index.js', b); console.log(a); `, ); - event = await getNextBuildSuccess(b); + let event = await getNextBuildSuccess(b); assertChangedAssets(event.changedAssets.size, 2); assertTimesBundled(defaultBundlerSpy.callCount, 2); - let result = await b.run(); let contents = await overlayFS.readFile( - result.bundleGraph.getBundles()[0].filePath, + event.bundleGraph.getBundles()[0].filePath, 'utf8', ); - assert( - contents.includes(`console.log("index.js", (0, _bDefault.default));`), - ); + assert(contents.includes(`console.log("index.js", (0, _b.b));`)); } finally { if (subscription) { await subscription.unsubscribe(); @@ -529,34 +526,32 @@ console.log(a); await overlayFS.mkdirp(fixture); subscription = await b.watch(); - let event = await getNextBuildSuccess(b); + await getNextBuildSuccess(b); assertTimesBundled(defaultBundlerSpy.callCount, 1); await overlayFS.writeFile( path.join(fixture, 'index.js'), - `import a from './a'; + `import {a} from './a'; import './a.css'; console.log(a); `, ); - event = await getNextBuildSuccess(b); + let event = await getNextBuildSuccess(b); assertChangedAssets(event.changedAssets.size, 2); assertTimesBundled(defaultBundlerSpy.callCount, 2); - let result = await b.run(); - // one CSS and one JS bundle - assert.equal(result.bundleGraph.getBundles().length, 2); + assert.equal(event.bundleGraph.getBundles().length, 2); let contents = await overlayFS.readFile( - result.bundleGraph.getBundles()[0].filePath, + event.bundleGraph.getBundles()[0].filePath, 'utf8', ); - assert(contents.includes(`console.log((0, _aDefault.default));`)); + assert(contents.includes(`console.log((0, _a.a));`)); - let bundleCSS = result.bundleGraph.getBundles()[1]; + let bundleCSS = event.bundleGraph.getBundles()[1]; assert.equal(bundleCSS.type, 'css'); let cssContent = await overlayFS.readFile(bundleCSS.filePath, 'utf8'); @@ -582,35 +577,33 @@ console.log(a); await overlayFS.mkdirp(fixture); subscription = await b.watch(); - let event = await getNextBuildSuccess(b); + await getNextBuildSuccess(b); assertTimesBundled(defaultBundlerSpy.callCount, 1); await overlayFS.writeFile( path.join(fixture, 'index.js'), - `import a from './a'; + `import {a} from './a'; const b = import('./b'); -console.log(a); +console.log(a, b); `, ); - event = await getNextBuildSuccess(b); + let event = await getNextBuildSuccess(b); let assets = Array.from(event.changedAssets.values()); assertChangedAssets(getChangedAssetsBeforeRuntimes(assets).length, 2); assertTimesBundled(defaultBundlerSpy.callCount, 2); - let result = await b.run(); - // original bundle and new dynamic import bundle JS bundle - assert.equal(result.bundleGraph.getBundles().length, 2); + assert.equal(event.bundleGraph.getBundles().length, 2); let contents = await overlayFS.readFile( - result.bundleGraph.getBundles()[0].filePath, + event.bundleGraph.getBundles()[0].filePath, 'utf8', ); - assert(contents.includes(`console.log((0, _aDefault.default));`)); + assert(contents.includes(`console.log((0, _a.a), b);`)); - let dynamicBundle = result.bundleGraph.getBundles()[1]; + let dynamicBundle = event.bundleGraph.getBundles()[1]; assert.equal(dynamicBundle.type, 'js'); let dynamicContent = await overlayFS.readFile( diff --git a/packages/core/integration-tests/test/integration/incremental-bundling/index-multi-symbol.js b/packages/core/integration-tests/test/integration/incremental-bundling/index-multi-symbol.js index dcba8c47127..10850f6a9a1 100644 --- a/packages/core/integration-tests/test/integration/incremental-bundling/index-multi-symbol.js +++ b/packages/core/integration-tests/test/integration/incremental-bundling/index-multi-symbol.js @@ -1,4 +1,4 @@ -import {a,b} from './multi-symbol-util.js'; +import {a, b} from './multi-symbol-util.js'; console.log('index.js', b); console.log(a); diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier-node-modules/a.js b/packages/core/integration-tests/test/integration/js-unused-import-specifier-node-modules/a.js similarity index 100% rename from packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier-node-modules/a.js rename to packages/core/integration-tests/test/integration/js-unused-import-specifier-node-modules/a.js diff --git a/packages/core/integration-tests/test/integration/js-unused-import-specifier-node-modules/node_modules/library/index.js b/packages/core/integration-tests/test/integration/js-unused-import-specifier-node-modules/node_modules/library/index.js new file mode 100644 index 00000000000..30ce056037b --- /dev/null +++ b/packages/core/integration-tests/test/integration/js-unused-import-specifier-node-modules/node_modules/library/index.js @@ -0,0 +1,3 @@ +export default used + 1; + +import {used, unused} from "./other.js"; diff --git a/packages/core/integration-tests/test/integration/js-unused-import-specifier-node-modules/node_modules/library/other.js b/packages/core/integration-tests/test/integration/js-unused-import-specifier-node-modules/node_modules/library/other.js new file mode 100644 index 00000000000..2b4d4e65809 --- /dev/null +++ b/packages/core/integration-tests/test/integration/js-unused-import-specifier-node-modules/node_modules/library/other.js @@ -0,0 +1 @@ +export const used = 122; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier/a.js b/packages/core/integration-tests/test/integration/js-unused-import-specifier/a.js similarity index 100% rename from packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier/a.js rename to packages/core/integration-tests/test/integration/js-unused-import-specifier/a.js index 67bfaf45f84..e50be408fe2 100644 --- a/packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier/a.js +++ b/packages/core/integration-tests/test/integration/js-unused-import-specifier/a.js @@ -1,3 +1,3 @@ -import {used, unused} from "./b.js"; - output = used; + +import {used, unused} from "./b.js"; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier/b.js b/packages/core/integration-tests/test/integration/js-unused-import-specifier/b.js similarity index 100% rename from packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier/b.js rename to packages/core/integration-tests/test/integration/js-unused-import-specifier/b.js diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier-node-modules/node_modules/library/index.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier-node-modules/node_modules/library/index.js deleted file mode 100644 index aba0af07de0..00000000000 --- a/packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier-node-modules/node_modules/library/index.js +++ /dev/null @@ -1,3 +0,0 @@ -import {a, b} from "./other.js"; - -export default a + 1; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier-node-modules/node_modules/library/other.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier-node-modules/node_modules/library/other.js deleted file mode 100644 index ac9e3327367..00000000000 --- a/packages/core/integration-tests/test/integration/scope-hoisting/es6/unused-import-specifier-node-modules/node_modules/library/other.js +++ /dev/null @@ -1 +0,0 @@ -export const a = 122; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-dependency-add/index.3.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-dependency-add/index.3.js new file mode 100644 index 00000000000..d578079f270 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-dependency-add/index.3.js @@ -0,0 +1,3 @@ +import {b, d} from "./library/b.js"; + +output = [b, d]; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/a.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/a.js new file mode 100644 index 00000000000..2dee3b29273 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/a.js @@ -0,0 +1,3 @@ +import { foo } from "./b.js"; + +output = foo; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/b.1.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/b.1.js new file mode 100644 index 00000000000..5b66a5942ca --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/b.1.js @@ -0,0 +1 @@ +export const foo = 123; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/b.2.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/b.2.js new file mode 100644 index 00000000000..45dd9d2490b --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/update-used-symbols-remove-export/b.2.js @@ -0,0 +1 @@ +export const bar = 123; diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js index c93d98c8922..ed33f7a71da 100644 --- a/packages/core/integration-tests/test/javascript.js +++ b/packages/core/integration-tests/test/javascript.js @@ -6269,6 +6269,7 @@ describe('javascript', function () { defaultTargetOptions: { shouldScopeHoist, }, + mode: 'production', }; let usesSymbolPropagation = shouldScopeHoist; describe(`sideEffects: false with${ @@ -6370,13 +6371,7 @@ describe('javascript', function () { type: 'js', assets: usesSymbolPropagation ? ['a.js', 'message1.js'] - : [ - 'a.js', - 'esmodule-helpers.js', - 'index.js', - 'message1.js', - 'message3.js', - ], + : ['a.js', 'esmodule-helpers.js', 'index.js', 'message1.js'], }, ]); @@ -6398,7 +6393,7 @@ describe('javascript', function () { assert.deepEqual( calls, - shouldScopeHoist ? ['message1'] : ['message1', 'message3', 'index'], + shouldScopeHoist ? ['message1'] : ['message1', 'index'], ); assert.deepEqual(res.output, 'Message 1'); }); @@ -6446,9 +6441,7 @@ describe('javascript', function () { assert.deepEqual( calls, - shouldScopeHoist - ? ['message2'] - : ['message1', 'message2', 'message3', 'index'], + shouldScopeHoist ? ['message2'] : ['message2', 'index'], ); assert.deepEqual(res.output, 'Message 2'); }); @@ -6467,13 +6460,7 @@ describe('javascript', function () { type: 'js', assets: usesSymbolPropagation ? ['c.js', 'message3.js'] - : [ - 'c.js', - 'esmodule-helpers.js', - 'index.js', - 'message1.js', - 'message3.js', - ], + : ['c.js', 'esmodule-helpers.js', 'index.js', 'message3.js'], }, ]); @@ -6494,7 +6481,7 @@ describe('javascript', function () { assert.deepEqual( calls, - shouldScopeHoist ? ['message3'] : ['message1', 'message3', 'index'], + shouldScopeHoist ? ['message3'] : ['message3', 'index'], ); assert.deepEqual(res.output, {default: 'Message 3'}); }); @@ -6525,10 +6512,7 @@ describe('javascript', function () { {require: false}, ); - assert.deepEqual( - calls, - shouldScopeHoist ? ['index'] : ['message1', 'message3', 'index'], - ); + assert.deepEqual(calls, ['index']); assert.deepEqual(res.output, 'Message 4'); }); @@ -6564,7 +6548,7 @@ describe('javascript', function () { assert.deepEqual(calls, ['foo', 'key', 'index']); } } else { - assert.deepEqual(calls, ['key', 'foo', 'bar', 'types', 'index']); + assert.deepEqual(calls, ['key', 'foo', 'types', 'index']); } assert.deepEqual(res.output, ['key', 'foo']); @@ -6700,8 +6684,8 @@ describe('javascript', function () { ); assert(!contents.includes('$import$')); - assert(contents.includes('= 1234;')); - assert(!contents.includes('= 5678;')); + assert(/=\s*1234/.test(contents)); + assert(!/=\s*5678/.test(contents)); let output = await run(b); assert.deepEqual(output, [1234, {default: 1234}]); @@ -6991,16 +6975,7 @@ describe('javascript', function () { assert.deepEqual( calls, - shouldScopeHoist - ? ['message1'] - : [ - 'message1', - 'message2', - 'message', - 'symbol1', - 'symbol2', - 'symbol', - ], + shouldScopeHoist ? ['message1'] : ['message1', 'message'], ); assert.deepEqual(res.output, 'Message 1'); }); @@ -7029,10 +7004,7 @@ describe('javascript', function () { {require: false}, ); - assert.deepEqual( - calls, - usesSymbolPropagation ? ['index'] : ['other', 'index'], - ); + assert.deepEqual(calls, ['index']); assert.deepEqual(res.output, 'Message 1'); }); @@ -7062,7 +7034,7 @@ describe('javascript', function () { calls, shouldScopeHoist ? ['message1'] - : ['message1', 'message2', 'message', 'index2', 'index'], + : ['message1', 'message', 'index2', 'index'], ); assert.deepEqual(res.output, 'Message 1'); }); @@ -7290,7 +7262,7 @@ describe('javascript', function () { let [v, async] = res; - assert.deepEqual(calls, shouldScopeHoist ? ['b'] : ['a', 'b', 'index']); + assert.deepEqual(calls, shouldScopeHoist ? ['b'] : ['b', 'index']); assert.deepEqual(v, 2); v = await async(); @@ -7298,7 +7270,7 @@ describe('javascript', function () { calls, shouldScopeHoist ? ['b', 'a', 'index', 'dynamic'] - : ['a', 'b', 'index', 'dynamic'], + : ['b', 'index', 'a', 'dynamic'], ); assert.deepEqual(v.default, [1, 3]); }); @@ -7327,7 +7299,7 @@ describe('javascript', function () { assert.deepEqual( calls, - shouldScopeHoist ? ['esm1'] : ['esm1', 'other', 'esm2', 'index'], + shouldScopeHoist ? ['esm1'] : ['esm1', 'index'], ); assert.deepEqual(res.output, 'Message 1'); }); @@ -7406,10 +7378,36 @@ describe('javascript', function () { ); assert.deepEqual( calls, - shouldScopeHoist ? ['commonjs'] : ['esm', 'commonjs', 'index'], + shouldScopeHoist ? ['commonjs'] : ['commonjs', 'index'], ); assert.deepEqual(res.output, 'Message 2'); }); }); + + it(`ignores missing unused import specifiers in source assets ${ + shouldScopeHoist ? 'with' : 'without' + } scope-hoisting`, async function () { + let b = await bundle( + path.join(__dirname, 'integration/js-unused-import-specifier/a.js'), + options, + ); + let res = await run(b, null, {require: false}); + assert.equal(res.output, 123); + }); + + it(`ignores missing unused import specifiers in node-modules ${ + shouldScopeHoist ? 'with' : 'without' + } scope-hoisting`, async function () { + let b = await bundle( + path.join( + __dirname, + '/integration/js-unused-import-specifier-node-modules/a.js', + ), + options, + ); + + let res = await run(b, null, {require: false}); + assert.equal(res.output, 123); + }); } }); diff --git a/packages/core/integration-tests/test/plugin.js b/packages/core/integration-tests/test/plugin.js index df8b80656ea..262400b4a9b 100644 --- a/packages/core/integration-tests/test/plugin.js +++ b/packages/core/integration-tests/test/plugin.js @@ -183,6 +183,7 @@ parcel-transformer-b`, defaultTargetOptions: { shouldScopeHoist: true, }, + mode: 'production', }, ); diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js index 683f228c156..87f925b737c 100644 --- a/packages/core/integration-tests/test/scope-hoisting.js +++ b/packages/core/integration-tests/test/scope-hoisting.js @@ -2209,6 +2209,7 @@ describe('scope hoisting', function () { __dirname, '/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js', ), + {mode: 'production'}, ); assertBundles(b, [ @@ -2218,6 +2219,7 @@ describe('scope hoisting', function () { 'entry.js', 'foo.js', 'bar.js', + 'bundle-manifest.js', 'bundle-url.js', 'cacheLoader.js', 'js-loader.js', @@ -2426,6 +2428,90 @@ describe('scope hoisting', function () { }); describe('correctly updates used symbols on changes', () => { + it('throws after removing an export', async function () { + let testDir = path.join( + __dirname, + '/integration/scope-hoisting/es6/update-used-symbols-remove-export', + ); + + let b = bundler(path.join(testDir, 'a.js'), { + inputFS: overlayFS, + outputFS: overlayFS, + }); + + await overlayFS.mkdirp(testDir); + await overlayFS.copyFile( + path.join(testDir, 'b.1.js'), + path.join(testDir, 'b.js'), + ); + + let subscription = await b.watch(); + + try { + let bundleEvent = await getNextBuild(b); + assert.strictEqual(bundleEvent.type, 'buildSuccess'); + let output = await run(bundleEvent.bundleGraph); + assert.deepEqual(output, 123); + + await overlayFS.copyFile( + path.join(testDir, 'b.2.js'), + path.join(testDir, 'b.js'), + ); + + bundleEvent = await getNextBuild(b); + assert.strictEqual(bundleEvent.type, 'buildFailure'); + let message = md`${normalizePath( + 'integration/scope-hoisting/es6/update-used-symbols-remove-export/b.js', + false, + )} does not export 'foo'`; + assert.deepEqual(bundleEvent.diagnostics, [ + { + message, + origin: '@parcel/core', + codeFrames: [ + { + filePath: path.join(testDir, 'a.js'), + language: 'js', + codeHighlights: [ + { + start: { + line: 1, + column: 10, + }, + end: { + line: 1, + column: 12, + }, + }, + ], + }, + ], + }, + ]); + + await overlayFS.copyFile( + path.join(testDir, 'b.1.js'), + path.join(testDir, 'b.js'), + ); + + bundleEvent = await getNextBuild(b); + assert.strictEqual(bundleEvent.type, 'buildSuccess'); + output = await run(bundleEvent.bundleGraph); + assert.deepEqual(output, 123); + + assert.deepStrictEqual( + new Set( + bundleEvent.bundleGraph.getUsedSymbols( + findAsset(bundleEvent.bundleGraph, 'b.js'), + ), + ), + new Set(['foo']), + ); + } finally { + await subscription.unsubscribe(); + } + }); + it('dependency symbols change', async function () { let testDir = path.join( __dirname, @@ -2486,7 +2572,7 @@ describe('scope hoisting', function () { } }); - it('add and remove dependency', async function () { + it('add and remove dependency (keep asset)', async function () { let testDir = path.join( __dirname, '/integration/scope-hoisting/es6/update-used-symbols-dependency-add', @@ -2567,6 +2653,107 @@ describe('scope hoisting', function () { } }); + it('add and remove dependency (remove asset)', async function () { + let testDir = path.join( + __dirname, + '/integration/scope-hoisting/es6/update-used-symbols-dependency-add', + ); + + let b = bundler(path.join(testDir, 'index.js'), { + inputFS: overlayFS, + outputFS: overlayFS, + }); + + await overlayFS.mkdirp(testDir); + await overlayFS.copyFile( + path.join(testDir, 'index.3.js'), + path.join(testDir, 'index.js'), + ); + + let subscription = await b.watch(); + + try { + let bundleEvent = await getNextBuild(b); + assert(bundleEvent.type === 'buildSuccess'); + let output = await run(bundleEvent.bundleGraph); + assert.deepEqual(output, [ + 789, + { + d1: 1, + d2: 2, + }, + ]); + + let assetC = nullthrows(findAsset(bundleEvent.bundleGraph, 'd1.js')); + assert.deepStrictEqual( + new Set(bundleEvent.bundleGraph.getUsedSymbols(assetC)), + new Set(['b']), + ); + let assetD = nullthrows(findAsset(bundleEvent.bundleGraph, 'd2.js')); + assert.deepStrictEqual( + new Set(bundleEvent.bundleGraph.getUsedSymbols(assetD)), + new Set(['*']), + ); + + await overlayFS.copyFile( + path.join(testDir, 'index.2.js'), + path.join(testDir, 'index.js'), + ); + + bundleEvent = await getNextBuild(b); + assert.strictEqual(bundleEvent.type, 'buildSuccess'); + output = await run(bundleEvent.bundleGraph); + assert.deepEqual(output, [ + 123, + 789, + { + d1: 1, + d2: 2, + }, + ]); + + assetC = nullthrows(findAsset(bundleEvent.bundleGraph, 'd1.js')); + assert.deepStrictEqual( + new Set(bundleEvent.bundleGraph.getUsedSymbols(assetC)), + new Set(['a', 'b']), + ); + assetD = nullthrows(findAsset(bundleEvent.bundleGraph, 'd2.js')); + assert.deepStrictEqual( + new Set(bundleEvent.bundleGraph.getUsedSymbols(assetD)), + new Set(['*']), + ); + + await overlayFS.copyFile( + path.join(testDir, 'index.3.js'), + path.join(testDir, 'index.js'), + ); + + bundleEvent = await getNextBuild(b); + assert.strictEqual(bundleEvent.type, 'buildSuccess'); + output = await run(bundleEvent.bundleGraph); + assert.deepEqual(output, [ + 789, + { + d1: 1, + d2: 2, + }, + ]); + + assetC = nullthrows(findAsset(bundleEvent.bundleGraph, 'd1.js')); + assert.deepStrictEqual( + new Set(bundleEvent.bundleGraph.getUsedSymbols(assetC)), + new Set(['b']), + ); + assetD = nullthrows(findAsset(bundleEvent.bundleGraph, 'd2.js')); + assert.deepStrictEqual( + new Set(bundleEvent.bundleGraph.getUsedSymbols(assetD)), + new Set(['*']), + ); + } finally { + await subscription.unsubscribe(); + } + }); + it('add and remove dependency with inline asset', async function () { let testDir = path.join( __dirname, @@ -2650,6 +2837,7 @@ describe('scope hoisting', function () { let b = bundler(path.join(testDir, 'index.html'), { inputFS: overlayFS, outputFS: overlayFS, + mode: 'production', }); await overlayFS.mkdirp(testDir); @@ -2756,29 +2944,6 @@ describe('scope hoisting', function () { await run(b); }); - it('ignores missing import specifiers in source assets', async function () { - let b = await bundle( - path.join( - __dirname, - 'integration/scope-hoisting/es6/unused-import-specifier/a.js', - ), - ); - let output = await run(b); - assert.equal(output, 123); - }); - - it('ignores unused import specifiers in node-modules', async function () { - let b = await bundle( - path.join( - __dirname, - '/integration/scope-hoisting/es6/unused-import-specifier-node-modules/a.js', - ), - ); - - let output = await run(b); - assert.equal(output, 123); - }); - it('can import urls to raw assets', async () => { let b = await bundle( path.join( diff --git a/packages/core/utils/src/collection.js b/packages/core/utils/src/collection.js index 0fb3349c1a3..1969dd3ae47 100644 --- a/packages/core/utils/src/collection.js +++ b/packages/core/utils/src/collection.js @@ -34,7 +34,10 @@ function sortEntry(entry: mixed) { return entry; } -export function setDifference(a: Set, b: Set): Set { +export function setDifference( + a: $ReadOnlySet, + b: $ReadOnlySet, +): Set { let difference = new Set(); for (let e of a) { if (!b.has(e)) { @@ -49,7 +52,7 @@ export function setDifference(a: Set, b: Set): Set { return difference; } -export function setIntersect(a: Set, b: Set): void { +export function setIntersect(a: Set, b: $ReadOnlySet): void { for (let entry of a) { if (!b.has(entry)) { a.delete(entry); @@ -61,7 +64,7 @@ export function setUnion(a: Iterable, b: Iterable): Set { return new Set([...a, ...b]); } -export function setEqual(a: Set, b: Set): boolean { +export function setEqual(a: $ReadOnlySet, b: $ReadOnlySet): boolean { if (a.size != b.size) { return false; } diff --git a/packages/core/utils/src/hash.js b/packages/core/utils/src/hash.js index d166761fe97..16b52f361d5 100644 --- a/packages/core/utils/src/hash.js +++ b/packages/core/utils/src/hash.js @@ -29,6 +29,21 @@ export function hashObject(obj: {+[string]: mixed, ...}): string { return hashString(JSON.stringify(objectSortedEntriesDeep(obj))); } +let testCache: {|[string]: Promise|} = { + /*:: ...null */ +}; export function hashFile(fs: FileSystem, filePath: string): Promise { + if (process.env.PARCEL_BUILD_ENV === 'test') { + // Development builds of these native modules are especially big and slow to hash. + if ( + /parcel-swc\.[^\\/]+\.node$|lightningcss.[^\\/]+.node$/.test(filePath) + ) { + let cacheEntry = testCache[filePath]; + if (cacheEntry) return cacheEntry; + let v = hashStream(fs.createReadStream(filePath)); + testCache[filePath] = v; + return v; + } + } return hashStream(fs.createReadStream(filePath)); } diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs index 370a2b253a1..102273f4e8e 100644 --- a/packages/transformers/js/core/src/hoist.rs +++ b/packages/transformers/js/core/src/hoist.rs @@ -1141,13 +1141,16 @@ pub struct Collect { pub has_cjs_exports: bool, pub is_esm: bool, pub should_wrap: bool, - // local name -> descriptor + /// local variable binding -> descriptor pub imports: HashMap, - // exported name -> descriptor + /// exported name -> descriptor pub exports: HashMap, - // local name -> exported name + /// local variable binding -> exported name pub exports_locals: HashMap, + /// source of the export-all --> location pub exports_all: HashMap, + /// the keys in `imports` that are actually used (referenced) + pub used_imports: HashSet, pub non_static_access: HashMap>, pub non_const_bindings: HashMap>, pub non_static_requires: HashSet, @@ -1188,6 +1191,9 @@ pub struct CollectResult { imports: Vec, exports: Vec, exports_all: Vec, + should_wrap: bool, + has_cjs_exports: bool, + is_esm: bool, } impl Collect { @@ -1211,6 +1217,7 @@ impl Collect { exports: HashMap::new(), exports_locals: HashMap::new(), exports_all: HashMap::new(), + used_imports: HashSet::new(), non_static_access: HashMap::new(), non_const_bindings: HashMap::new(), non_static_requires: HashSet::new(), @@ -1227,6 +1234,31 @@ impl Collect { impl From for CollectResult { fn from(collect: Collect) -> CollectResult { + let imports = collect + .imports + .into_iter() + .filter(|(local, _)| { + collect.used_imports.contains(local) || collect.exports_locals.contains_key(local) + }) + .map( + |( + local, + Import { + source, + specifier, + loc, + kind, + }, + )| CollectImportedSymbol { + source, + local: local.0, + imported: specifier, + loc, + kind, + }, + ) + .collect(); + let mut exports: Vec = collect .exports .into_iter() @@ -1263,33 +1295,16 @@ impl From for CollectResult { } CollectResult { - imports: collect - .imports - .into_iter() - .map( - |( - local, - Import { - source, - specifier, - loc, - kind, - }, - )| CollectImportedSymbol { - source, - local: local.0, - imported: specifier, - loc, - kind, - }, - ) - .collect(), + imports, exports, exports_all: collect .exports_all .into_iter() .map(|(source, loc)| CollectExportedAll { source, loc }) .collect(), + should_wrap: collect.should_wrap, + has_cjs_exports: collect.has_cjs_exports, + is_esm: collect.is_esm, } } } @@ -1299,7 +1314,17 @@ impl Visit for Collect { self.in_module_this = true; self.in_top_level = true; self.in_function = false; - node.visit_children_with(self); + // Visit all imports first so that all imports are known when collecting used_imports + for n in &node.body { + if n.is_module_decl() { + n.visit_with(self); + } + } + for n in &node.body { + if !n.is_module_decl() { + n.visit_with(self); + } + } self.in_module_this = false; if let Some(bailouts) = &mut self.bailouts { @@ -1783,6 +1808,10 @@ impl Visit for Collect { .entry(id!(ident)) .or_default() .push(ident.span); + + if self.imports.contains_key(&id!(ident)) { + self.used_imports.insert(id!(ident)); + } } _ => { node.visit_children_with(self); diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js index 88278798cd9..5fd07c5daf1 100644 --- a/packages/transformers/js/src/JSTransformer.js +++ b/packages/transformers/js/src/JSTransformer.js @@ -854,6 +854,27 @@ export default (new Transformer({ dep.symbols.ensure(); dep.symbols.set('*', '*', convertLoc(loc), true); } + + // Add * symbol if there are CJS exports, no imports/exports at all, or the asset is wrapped. + // This allows accessing symbols that don't exist without errors in symbol propagation. + if ( + symbol_result.has_cjs_exports || + (!symbol_result.is_esm && + deps.size === 0 && + symbol_result.exports.length === 0) || + (symbol_result.should_wrap && !asset.symbols.hasExportSymbol('*')) + ) { + asset.symbols.set('*', `$`); + } + + // For dynamic imports, mark everything as imported (a more detailed analysis similar to + // what scope hoisting does with `const {foo} = await import(...);` isn't hook up yet.) + for (let d of deps.values()) { + if (d.priority === 'lazy') { + d.symbols.ensure(); + d.symbols.set('*', '$'); + } + } } if (needs_esm_helpers) { diff --git a/packages/utils/node-resolver-core/package.json b/packages/utils/node-resolver-core/package.json index c18447dc9fd..3201d17f85a 100644 --- a/packages/utils/node-resolver-core/package.json +++ b/packages/utils/node-resolver-core/package.json @@ -63,4 +63,4 @@ "util": "^0.12.3", "vm-browserify": "^1.1.2" } -} \ No newline at end of file +}