From 3092d3c167b1da72f1b949cb96de6e0a0b36dc75 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 9 Sep 2022 07:23:03 +0200 Subject: [PATCH] Code splitting across reexports using symbol data by splitting dependencies (#8432) --- packages/core/core/src/AssetGraph.js | 2 +- packages/core/core/src/BundleGraph.js | 193 +++++++++++++++--- packages/core/core/src/dumpGraphToGraphViz.js | 24 ++- packages/core/core/src/public/Symbols.js | 1 - .../core/src/requests/AssetGraphRequest.js | 158 ++++++++++---- packages/core/core/src/types.js | 11 +- .../integration/resolver-canDefer/index.js | 4 +- .../resolver-canDefer/library/index.js | 1 + .../es6/codesplit-reexports/library/bar.js | 1 + .../es6/codesplit-reexports/library/bar2.js | 1 + .../es6/codesplit-reexports/library/foo.js | 1 + .../es6/codesplit-reexports/library/foo2.js | 2 + .../es6/codesplit-reexports/library/index.js | 7 + .../codesplit-reexports/library/package.json | 4 + .../es6/codesplit-reexports/src/async.js | 3 + .../es6/codesplit-reexports/src/entry.js | 3 + .../index.js | 3 + .../library/a.js | 2 + .../library/b.js | 2 + .../library/dynamic.js | 4 + .../library/index.js | 4 + .../library/package.json | 3 + .../core/integration-tests/test/javascript.js | 122 ++++++++--- .../core/integration-tests/test/plugin.js | 14 +- .../integration-tests/test/scope-hoisting.js | 93 ++++----- packages/core/test-utils/src/utils.js | 10 +- .../packagers/js/src/ScopeHoistingPackager.js | 111 +++++----- .../src/ServiceWorkerRuntime.js | 3 +- 28 files changed, 550 insertions(+), 237 deletions(-) create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar2.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo2.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/index.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/package.json create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/async.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/index.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/a.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/b.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/dynamic.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/index.js create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/package.json diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index 5afb1f68e1e..534d7f0b263 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -56,7 +56,7 @@ export function nodeFromDep(dep: Dependency): DependencyNode { deferred: false, excluded: false, usedSymbolsDown: new Set(), - usedSymbolsUp: new Set(), + usedSymbolsUp: new Map(), usedSymbolsDownDirty: true, usedSymbolsUpDirtyDown: true, usedSymbolsUpDirtyUp: true, diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index b0ab7d12864..04dfabbd4bc 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -7,6 +7,7 @@ import type { TraversalActions, } from '@parcel/types'; import type { + ContentKey, ContentGraphOpts, NodeId, SerializedContentGraph, @@ -31,7 +32,7 @@ import invariant from 'assert'; import nullthrows from 'nullthrows'; import {ContentGraph, ALL_EDGE_TYPES, mapVisitor} from '@parcel/graph'; import {Hash, hashString} from '@parcel/hash'; -import {objectSortedEntriesDeep, getRootDir} from '@parcel/utils'; +import {DefaultMap, objectSortedEntriesDeep, getRootDir} from '@parcel/utils'; import {Priority, BundleBehavior, SpecifierType} from './types'; import {getBundleGroupId, getPublicId} from './utils'; @@ -146,7 +147,8 @@ export default class BundleGraph { assetPublicIds: Set = new Set(), ): BundleGraph { let graph = new ContentGraph(); - let assetGroupIds = new Set(); + let assetGroupIds = new Map(); + let dependencies = new Map(); let assetGraphNodeIdToBundleGraphNodeId = new Map(); let assetGraphRootNode = @@ -168,50 +170,175 @@ export default class BundleGraph { publicIdByAssetId.set(assetId, publicId); assetPublicIds.add(publicId); } + } else if (node.type === 'asset_group') { + assetGroupIds.set(nodeId, assetGraph.getNodeIdsConnectedFrom(nodeId)); } + } + + let walkVisited = new Set(); + function walk(nodeId) { + if (walkVisited.has(nodeId)) return; + walkVisited.add(nodeId); + let node = nullthrows(assetGraph.getNode(nodeId)); + if (node.type === 'dependency' && node.value.symbols != null) { + // asset -> symbols that should be imported directly from that asset + let targets = new DefaultMap>( + () => new Map(), + ); + let externalSymbols = new Set(); + let hasAmbiguousSymbols = false; + + for (let [symbol, resolvedSymbol] of node.usedSymbolsUp) { + if (resolvedSymbol) { + targets + .get(resolvedSymbol.asset) + .set(symbol, resolvedSymbol.symbol ?? symbol); + } else if (resolvedSymbol === null) { + externalSymbols.add(symbol); + } else if (resolvedSymbol === undefined) { + hasAmbiguousSymbols = true; + break; + } + } + + if ( + // Only perform rewriting when there is an imported symbol + // - If the target is side-effect-free, the symbols point to the actual target and removing + // the original dependency resolution is fine + // - Otherwise, keep this dependency unchanged for its potential side effects + node.usedSymbolsUp.size > 0 && + // Only perform rewriting if the dependency only points to a single asset (e.g. CSS modules) + !hasAmbiguousSymbols && + // TODO We currently can't rename imports in async imports, e.g. from + // (parcelRequire("...")).then(({ a }) => a); + // to + // (parcelRequire("...")).then(({ a: b }) => a); + // or + // (parcelRequire("...")).then((a)=>a); + // if the reexporting asset did `export {a as b}` or `export * as a` + node.value.priority === Priority.sync + ) { + // TODO adjust sourceAssetIdNode.value.dependencies ? + let deps = [ + // Keep the original dependency + { + asset: null, + dep: graph.addNodeByContentKey(node.id, { + ...node, + value: { + ...node.value, + symbols: node.value.symbols + ? new Map( + [...node.value.symbols].filter(([k]) => + externalSymbols.has(k), + ), + ) + : undefined, + }, + usedSymbolsUp: new Map( + [...node.usedSymbolsUp].filter(([k]) => + externalSymbols.has(k), + ), + ), + usedSymbolsDown: new Set(), + excluded: externalSymbols.size === 0, + }), + }, + ...[...targets].map(([asset, target]) => { + let newNodeId = hashString( + node.id + [...target.keys()].join(','), + ); + return { + asset, + dep: graph.addNodeByContentKey(newNodeId, { + ...node, + id: newNodeId, + value: { + ...node.value, + id: newNodeId, + symbols: node.value.symbols + ? new Map( + [...node.value.symbols] + .filter(([k]) => target.has(k) || k === '*') + .map(([k, v]) => [target.get(k) ?? k, v]), + ) + : undefined, + }, + usedSymbolsUp: new Map( + [...node.usedSymbolsUp] + .filter(([k]) => target.has(k) || k === '*') + .map(([k, v]) => [target.get(k) ?? k, v]), + ), + usedSymbolsDown: new Set(), + }), + }; + }), + ]; + dependencies.set(nodeId, deps); + + // Jump to the dependencies that are used in this dependency + for (let id of targets.keys()) { + walk(assetGraph.getNodeIdByContentKey(id)); + } + return; + } else { + // No special handling + let bundleGraphNodeId = graph.addNodeByContentKey(node.id, node); + assetGraphNodeIdToBundleGraphNodeId.set(nodeId, bundleGraphNodeId); + } + } // Don't copy over asset groups into the bundle graph. - if (node.type === 'asset_group') { - assetGroupIds.add(nodeId); - } else { + else if (node.type !== 'asset_group') { let bundleGraphNodeId = graph.addNodeByContentKey(node.id, node); if (node.id === assetGraphRootNode?.id) { graph.setRootNodeId(bundleGraphNodeId); } assetGraphNodeIdToBundleGraphNodeId.set(nodeId, bundleGraphNodeId); } + + for (let id of assetGraph.getNodeIdsConnectedFrom(nodeId)) { + walk(id); + } } + walk(nullthrows(assetGraph.rootNodeId)); for (let edge of assetGraph.getAllEdges()) { - let fromIds; if (assetGroupIds.has(edge.from)) { - fromIds = [ - ...assetGraph.getNodeIdsConnectedTo( - edge.from, - bundleGraphEdgeTypes.null, - ), - ]; - } else { - fromIds = [edge.from]; + continue; } - - for (let from of fromIds) { - if (assetGroupIds.has(edge.to)) { - for (let to of assetGraph.getNodeIdsConnectedFrom( - edge.to, - bundleGraphEdgeTypes.null, - )) { + if (dependencies.has(edge.from)) { + // Discard previous edge, insert outgoing edges for all split dependencies + for (let {asset, dep} of nullthrows(dependencies.get(edge.from))) { + if (asset != null) { graph.addEdge( - nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(from)), - nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(to)), + dep, + nullthrows( + assetGraphNodeIdToBundleGraphNodeId.get( + assetGraph.getNodeIdByContentKey(asset), + ), + ), ); } - } else { - graph.addEdge( - nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(from)), - nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(edge.to)), - ); } + continue; + } + if (!assetGraphNodeIdToBundleGraphNodeId.has(edge.from)) { + continue; + } + + let to: Array = dependencies.get(edge.to)?.map(v => v.dep) ?? + assetGroupIds + .get(edge.to) + ?.map(id => + nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(id)), + ) ?? [nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(edge.to))]; + + for (let t of to) { + graph.addEdge( + nullthrows(assetGraphNodeIdToBundleGraphNodeId.get(edge.from)), + t, + ); } } @@ -1737,16 +1864,15 @@ export default class BundleGraph { let node = this._graph.getNodeByContentKey(asset.id); invariant(node && node.type === 'asset'); return this._symbolPropagationRan - ? makeReadOnlySet(node.usedSymbols) + ? makeReadOnlySet(new Set(node.usedSymbols.keys())) : null; } getUsedSymbolsDependency(dep: Dependency): ?$ReadOnlySet { let node = this._graph.getNodeByContentKey(dep.id); invariant(node && node.type === 'dependency'); - return this._symbolPropagationRan - ? makeReadOnlySet(node.usedSymbolsUp) - : null; + let result = new Set(node.usedSymbolsUp.keys()); + return this._symbolPropagationRan ? makeReadOnlySet(result) : null; } merge(other: BundleGraph) { @@ -1758,6 +1884,7 @@ export default class BundleGraph { let existingNode = nullthrows(this._graph.getNode(existingNodeId)); // Merge symbols, recompute dep.exluded based on that + if (existingNode.type === 'asset') { invariant(otherNode.type === 'asset'); existingNode.usedSymbols = new Set([ @@ -1770,7 +1897,7 @@ export default class BundleGraph { ...existingNode.usedSymbolsDown, ...otherNode.usedSymbolsDown, ]); - existingNode.usedSymbolsUp = new Set([ + existingNode.usedSymbolsUp = new Map([ ...existingNode.usedSymbolsUp, ...otherNode.usedSymbolsUp, ]); diff --git a/packages/core/core/src/dumpGraphToGraphViz.js b/packages/core/core/src/dumpGraphToGraphViz.js index 79519fa9183..219a64a034d 100644 --- a/packages/core/core/src/dumpGraphToGraphViz.js +++ b/packages/core/core/src/dumpGraphToGraphViz.js @@ -21,11 +21,14 @@ const COLORS = { }; const TYPE_COLORS = { + // bundle graph bundle: 'blue', contains: 'grey', internal_async: 'orange', references: 'red', sibling: 'green', + // asset graph + // request graph invalidated_by_create: 'green', invalidated_by_create_above: 'orange', invalidate_by_update: 'cyan', @@ -80,8 +83,13 @@ export default async function dumpGraphToGraphViz( if (node.type === 'dependency') { label += node.value.specifier; let parts = []; - if (node.value.priority !== Priority.sync) - parts.push(node.value.priority); + if (node.value.priority !== Priority.sync) { + parts.push( + Object.entries(Priority).find( + ([, v]) => v === node.value.priority, + )?.[0], + ); + } if (node.value.isOptional) parts.push('optional'); if (node.value.specifierType === SpecifierType.url) parts.push('url'); if (node.hasDeferred) parts.push('deferred'); @@ -103,7 +111,17 @@ export default async function dumpGraphToGraphViz( label += '\\nweakSymbols: ' + weakSymbols.join(','); } if (node.usedSymbolsUp.size > 0) { - label += '\\nusedSymbolsUp: ' + [...node.usedSymbolsUp].join(','); + label += + '\\nusedSymbolsUp: ' + + [...node.usedSymbolsUp] + .map(([s, sAsset]) => + sAsset + ? `${s}(${sAsset.asset}.${sAsset.symbol ?? ''})` + : sAsset === null + ? `${s}(external)` + : `${s}(ambiguous)`, + ) + .join(','); } if (node.usedSymbolsDown.size > 0) { label += diff --git a/packages/core/core/src/public/Symbols.js b/packages/core/core/src/public/Symbols.js index 82ae6e3ae70..9d986015485 100644 --- a/packages/core/core/src/public/Symbols.js +++ b/packages/core/core/src/public/Symbols.js @@ -27,7 +27,6 @@ const EMPTY_ITERATOR = { const inspect = Symbol.for('nodejs.util.inspect.custom'); let valueToSymbols: WeakMap = new WeakMap(); - export class AssetSymbols implements IAssetSymbols { /*:: @@iterator(): Iterator<[ISymbol, {|local: ISymbol, loc: ?SourceLocation, meta?: ?Meta|}]> { return ({}: any); } diff --git a/packages/core/core/src/requests/AssetGraphRequest.js b/packages/core/core/src/requests/AssetGraphRequest.js index e9ebc26898b..02d46e57d40 100644 --- a/packages/core/core/src/requests/AssetGraphRequest.js +++ b/packages/core/core/src/requests/AssetGraphRequest.js @@ -1,7 +1,7 @@ // @flow strict-local import type {Diagnostic} from '@parcel/diagnostic'; -import type {NodeId} from '@parcel/graph'; +import type {ContentKey, NodeId} from '@parcel/graph'; import type {Async, Symbol, Meta} from '@parcel/types'; import type {SharedReference} from '@parcel/workers'; import type { @@ -38,8 +38,7 @@ import { fromProjectPathRelative, fromProjectPath, } from '../projectPath'; - -import dumpToGraphViz from '../dumpGraphToGraphViz'; +import dumpGraphToGraphViz from '../dumpGraphToGraphViz'; type AssetGraphRequestInput = {| entries?: Array, @@ -209,14 +208,21 @@ export class AssetGraphBuilder { d => d.value.env.shouldScopeHoist, ); if (this.assetGraph.symbolPropagationRan) { + await dumpGraphToGraphViz( + this.assetGraph, + 'AssetGraph_' + this.name + '_before_prop', + ); try { this.propagateSymbols(); } catch (e) { - dumpToGraphViz(this.assetGraph, 'AssetGraph_' + this.name + '_failed'); + await dumpGraphToGraphViz( + this.assetGraph, + 'AssetGraph_' + this.name + '_failed', + ); throw e; } } - dumpToGraphViz(this.assetGraph, 'AssetGraph_' + this.name); + await dumpGraphToGraphViz(this.assetGraph, 'AssetGraph_' + this.name); return { assetGraph: this.assetGraph, @@ -408,7 +414,7 @@ export class AssetGraphBuilder { const logFallbackNamespaceInsertion = ( assetNode, - symbol, + symbol: Symbol, depNode1, depNode2, ) => { @@ -449,42 +455,53 @@ export class AssetGraphBuilder { } } - // the symbols that are reexport (not used in `asset`) -> the corresponding outgoingDep(s) - // There could be multiple dependencies with non-statically analyzable exports - let reexportedSymbols = new Map(); + // the symbols that are reexported (not used in `asset`) -> 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; - // excluded, assume everything that is requested exists - if ( + let isExcluded = this.assetGraph.getNodeIdsConnectedFrom( this.assetGraph.getNodeIdByContentKey(outgoingDep.id), - ).length === 0 - ) { - outgoingDep.usedSymbolsDown.forEach(s => - outgoingDep.usedSymbolsUp.add(s), + ).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(s => { + outgoingDep.usedSymbolsUp.forEach((sResolved, s) => { // If the symbol could come from multiple assets at runtime, assetNode's // namespace will be needed at runtime to perform the lookup on. - if (reexportedSymbols.has(s) && !assetNode.usedSymbols.has('*')) { - logFallbackNamespaceInsertion( - assetNode, - s, - nullthrows(reexportedSymbols.get(s)), - outgoingDep, - ); + 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); } - reexportedSymbols.set(s, outgoingDep); }); } - for (let s of outgoingDep.usedSymbolsUp) { + for (let [s, sResolved] of outgoingDep.usedSymbolsUp) { if (!outgoingDep.usedSymbolsDown.has(s)) { // usedSymbolsDown is a superset of usedSymbolsUp continue; @@ -500,16 +517,21 @@ export class AssetGraphBuilder { if (reexported != null) { reexported.forEach(s => { // see same code above - if (reexportedSymbols.has(s) && !assetNode.usedSymbols.has('*')) { - logFallbackNamespaceInsertion( - assetNode, - s, - nullthrows(reexportedSymbols.get(s)), - outgoingDep, - ); + 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); } - reexportedSymbols.set(s, outgoingDep); }); } } @@ -517,9 +539,27 @@ export class AssetGraphBuilder { let errors: Array = []; + function usedSymbolsUpAmbiguous(old, current, s, value) { + if (old.has(s)) { + let valueOld = old.get(s); + if ( + valueOld !== value && + !( + valueOld?.asset === value.asset && + valueOld?.symbol === value.symbol + ) + ) { + // The dependency points to multiple assets (via an asset group). + current.set(s, undefined); + return; + } + } + current.set(s, value); + } + for (let incomingDep of incomingDeps) { let incomingDepUsedSymbolsUpOld = incomingDep.usedSymbolsUp; - incomingDep.usedSymbolsUp = new Set(); + incomingDep.usedSymbolsUp = new Map(); let incomingDepSymbols = incomingDep.value.symbols; if (!incomingDepSymbols) continue; @@ -529,11 +569,34 @@ export class AssetGraphBuilder { assetSymbols == null || // Assume everything could be provided if symbols are cleared assetNode.value.bundleBehavior === BundleBehavior.isolated || assetNode.value.bundleBehavior === BundleBehavior.inline || - assetNode.usedSymbols.has(s) || - reexportedSymbols.has(s) || - s === '*' + s === '*' || + assetNode.usedSymbols.has(s) ) { - incomingDep.usedSymbolsUp.add(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( @@ -571,7 +634,7 @@ export class AssetGraphBuilder { } } - if (!equalSet(incomingDepUsedSymbolsUpOld, incomingDep.usedSymbolsUp)) { + if (!equalMap(incomingDepUsedSymbolsUpOld, incomingDep.usedSymbolsUp)) { changedDeps.add(incomingDep); incomingDep.usedSymbolsUpDirtyUp = true; } @@ -604,7 +667,9 @@ export class AssetGraphBuilder { // This ensures a consistent ordering of these symbols when packaging. // See https://github.com/parcel-bundler/parcel/pull/8212 for (let dep of changedDeps) { - dep.usedSymbolsUp = new Set([...dep.usedSymbolsUp].sort()); + dep.usedSymbolsUp = new Map( + [...dep.usedSymbolsUp].sort(([a], [b]) => a.localeCompare(b)), + ); } } @@ -893,6 +958,19 @@ export class AssetGraphBuilder { } } +function equalMap( + a: $ReadOnlyMap, + b: $ReadOnlyMap, +) { + if (a.size !== b.size) return false; + for (let [k, v] of a) { + if (!b.has(k)) return false; + let vB = b.get(k); + if (vB?.asset !== v?.asset || vB?.symbol !== v?.symbol) return false; + } + return true; +} + function equalSet(a: $ReadOnlySet, b: $ReadOnlySet) { return a.size === b.size && [...a].every(i => b.has(i)); } diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index 82382ad1295..1a23828aba0 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -307,7 +307,16 @@ export type DependencyNode = {| /** dependency was deferred (= no used symbols (in immediate parents) & side-effect free) */ hasDeferred?: boolean, usedSymbolsDown: Set, - usedSymbolsUp: Set, + /** + * a requested symbol -> either + * - if ambiguous (e.g. dependency to asset group with both CSS modules and JS asset): undefined + * - if external: null + * - the asset it resolved to, and the potentially renamed export name + */ + usedSymbolsUp: Map< + Symbol, + {|asset: ContentKey, symbol: ?Symbol|} | void | null, + >, /** for the "down" pass, the dependency resolution asset needs to be updated */ usedSymbolsDownDirty: boolean, /** for the "up" pass, the parent asset needs to be updated */ diff --git a/packages/core/integration-tests/test/integration/resolver-canDefer/index.js b/packages/core/integration-tests/test/integration/resolver-canDefer/index.js index f8f5ca007e9..381a65c3b7c 100644 --- a/packages/core/integration-tests/test/integration/resolver-canDefer/index.js +++ b/packages/core/integration-tests/test/integration/resolver-canDefer/index.js @@ -1,3 +1,3 @@ -import {a} from "./library"; +import {a, index} from "./library"; -output = a; +output = [a, index]; diff --git a/packages/core/integration-tests/test/integration/resolver-canDefer/library/index.js b/packages/core/integration-tests/test/integration/resolver-canDefer/library/index.js index cd00ec2d483..86922c23686 100644 --- a/packages/core/integration-tests/test/integration/resolver-canDefer/library/index.js +++ b/packages/core/integration-tests/test/integration/resolver-canDefer/library/index.js @@ -2,3 +2,4 @@ sideEffect("index"); export {a} from "./a.js"; export {b} from "./b.js"; export {c} from "./c.js"; +export const index = "Index"; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar.js new file mode 100644 index 00000000000..b8b030fb17b --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar.js @@ -0,0 +1 @@ +export const bar = 3; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar2.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar2.js new file mode 100644 index 00000000000..8a4703d5b88 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/bar2.js @@ -0,0 +1 @@ +export const bar2 = 30; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo.js new file mode 100644 index 00000000000..cc57a55e257 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo.js @@ -0,0 +1 @@ +export const foo = 2; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo2.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo2.js new file mode 100644 index 00000000000..dc8c6371bf0 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/foo2.js @@ -0,0 +1,2 @@ +export const foo2 = 20; + diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/index.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/index.js new file mode 100644 index 00000000000..760d6bb9174 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/index.js @@ -0,0 +1,7 @@ +export { foo } from "./foo"; +export * from "./bar"; + +export { foo2 as foo3 } from "./foo2"; +export { bar2 as bar3 } from "./bar2"; + +export const own = 3; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/package.json b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/package.json new file mode 100644 index 00000000000..a2dd4df2af0 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/library/package.json @@ -0,0 +1,4 @@ +{ + "name": "lib", + "sideEffects": false +} diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/async.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/async.js new file mode 100644 index 00000000000..09e18d5f44e --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/async.js @@ -0,0 +1,3 @@ +import { foo3, bar3 } from "../library"; + +export default [foo3, bar3]; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js new file mode 100644 index 00000000000..d5824a7266e --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js @@ -0,0 +1,3 @@ +import { foo, bar } from "../library"; + +output = import("./async").then(v => [v.default, [foo, bar]]) diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/index.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/index.js new file mode 100644 index 00000000000..3888e43d6b2 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/index.js @@ -0,0 +1,3 @@ +import { Context } from "./library"; + +output = [Context, () => import("./library/dynamic")]; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/a.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/a.js new file mode 100644 index 00000000000..9eda04b81ad --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/a.js @@ -0,0 +1,2 @@ +sideEffect("a"); +export const Ctx = 1; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/b.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/b.js new file mode 100644 index 00000000000..18b74695dea --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/b.js @@ -0,0 +1,2 @@ +sideEffect("b"); +export const Ctx = 2; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/dynamic.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/dynamic.js new file mode 100644 index 00000000000..74c81826df6 --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/dynamic.js @@ -0,0 +1,4 @@ +sideEffect("dynamic"); +import { Ctx } from "./a.js"; +import { id } from "./index.js"; +export default [Ctx, id]; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/index.js b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/index.js new file mode 100644 index 00000000000..21b2f50e4ab --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/index.js @@ -0,0 +1,4 @@ +sideEffect("index"); +export { Ctx } from "./a.js"; +export { Ctx as Context } from "./b.js"; +export const id = 3; diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/package.json b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/package.json new file mode 100644 index 00000000000..1b95642997c --- /dev/null +++ b/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/library/package.json @@ -0,0 +1,3 @@ +{ + "sideEffects": false +} diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js index 8cedde288af..6c25eb10a05 100644 --- a/packages/core/integration-tests/test/javascript.js +++ b/packages/core/integration-tests/test/javascript.js @@ -1684,7 +1684,7 @@ describe('javascript', function () { let bundles = b.getBundles(); let worker = bundles.find(b => b.env.isWorker()); let manifest, version; - await await runBundle(b, worker, { + await runBundle(b, worker, { output(m, v) { manifest = m; version = v; @@ -6246,7 +6246,7 @@ describe('javascript', function () { }, { type: 'js', - assets: ['index.js', 'a.js', 'b1.js'], + assets: ['index.js', 'b1.js'], }, { type: 'css', @@ -6291,7 +6291,7 @@ describe('javascript', function () { }, { type: 'js', - assets: ['index.js', 'a.js', 'b1.js'], + assets: ['index.js', 'b1.js'], }, ]); @@ -6320,7 +6320,21 @@ describe('javascript', function () { options, ); - assertDependencyWasExcluded(b, 'index.js', './message2.js'); + assertBundles(b, [ + { + type: 'js', + assets: usesSymbolPropagation + ? ['a.js', 'message1.js'] + : [ + 'a.js', + 'esmodule-helpers.js', + 'index.js', + 'message1.js', + 'message3.js', + ], + }, + ]); + if (usesSymbolPropagation) { // TODO this only excluded, but should be deferred. assert(!findAsset(b, 'message3.js')); @@ -6403,10 +6417,24 @@ describe('javascript', function () { options, ); + assertBundles(b, [ + { + type: 'js', + assets: usesSymbolPropagation + ? ['c.js', 'message3.js'] + : [ + 'c.js', + 'esmodule-helpers.js', + 'index.js', + 'message1.js', + 'message3.js', + ], + }, + ]); + if (usesSymbolPropagation) { assert(!findAsset(b, 'message1.js')); } - assertDependencyWasExcluded(b, 'index.js', './message2.js'); let calls = []; let res = await run( @@ -6483,12 +6511,17 @@ describe('javascript', function () { {require: false}, ); - assert.deepEqual( - calls, - shouldScopeHoist - ? ['key', 'foo', 'index'] - : ['key', 'foo', 'bar', 'types', 'index'], - ); + if (shouldScopeHoist) { + try { + assert.deepEqual(calls, ['key', 'foo', 'index']); + } catch (e) { + // A different dependency order, but this is deemed acceptable as it's sideeffect free + assert.deepEqual(calls, ['foo', 'key', 'index']); + } + } else { + assert.deepEqual(calls, ['key', 'foo', 'bar', 'types', 'index']); + } + assert.deepEqual(res.output, ['key', 'foo']); }); @@ -6707,11 +6740,9 @@ describe('javascript', function () { ); assert(!called, 'side effect called'); assert.deepEqual(res.output, 4); - assertDependencyWasExcluded( - bundleEvent.bundleGraph, - 'index.js', - './bar', - ); + if (usesSymbolPropagation) { + assert(!findAsset(bundleEvent.bundleGraph, 'index.js')); + } await overlayFS.mkdirp(path.join(testDir, 'node_modules/bar')); await overlayFS.copyFile( @@ -7082,10 +7113,7 @@ describe('javascript', function () { ); if (usesSymbolPropagation) { - assert.deepStrictEqual( - new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'index.js')))), - new Set([]), - ); + assert(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'other.js')))), new Set(['default']), @@ -7120,10 +7148,7 @@ describe('javascript', function () { ); if (usesSymbolPropagation) { - assert.deepStrictEqual( - new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'index.js')))), - new Set([]), - ); + assert(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'other.js')))), new Set(['bar']), @@ -7158,10 +7183,7 @@ describe('javascript', function () { ); if (usesSymbolPropagation) { - assert.deepStrictEqual( - new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'index.js')))), - new Set([]), - ); + assert(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'other.js')))), new Set(['default', 'bar']), @@ -7186,6 +7208,43 @@ describe('javascript', function () { assert.deepEqual(res.output, ['foo', 'bar']); }); + it('supports partially used reexporting index file', async function () { + let b = await bundle( + path.join( + __dirname, + '/integration/scope-hoisting/es6/side-effects-re-exports-partially-used/index.js', + ), + options, + ); + + let calls = []; + let res = ( + await run( + b, + { + sideEffect: caller => { + calls.push(caller); + }, + }, + {require: false}, + ) + ).output; + + let [v, async] = res; + + assert.deepEqual(calls, shouldScopeHoist ? ['b'] : ['a', 'b', 'index']); + assert.deepEqual(v, 2); + + v = await async(); + assert.deepEqual( + calls, + shouldScopeHoist + ? ['b', 'a', 'index', 'dynamic'] + : ['a', 'b', 'index', 'dynamic'], + ); + assert.deepEqual(v.default, [1, 3]); + }); + it('supports deferring non-weak dependencies that are not used', async function () { let b = await bundle( path.join( @@ -7269,17 +7328,12 @@ describe('javascript', function () { if (usesSymbolPropagation) { assert(!findAsset(b, 'esm.js')); + assert(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'commonjs.js')))), // the exports object is used freely new Set(['*', 'message2']), ); - assert.deepEqual( - new Set( - b.getUsedSymbols(findDependency(b, 'index.js', './commonjs.js')), - ), - new Set(['message2']), - ); } let calls = []; diff --git a/packages/core/integration-tests/test/plugin.js b/packages/core/integration-tests/test/plugin.js index 3310449a55a..df8b80656ea 100644 --- a/packages/core/integration-tests/test/plugin.js +++ b/packages/core/integration-tests/test/plugin.js @@ -10,7 +10,6 @@ import { bundler, distDir, findAsset, - findDependency, getNextBuild, outputFS as fs, overlayFS, @@ -187,10 +186,7 @@ parcel-transformer-b`, }, ); - assert.deepStrictEqual( - new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'index.js')))), - new Set([]), - ); + assert(!findAsset(b, 'index.js')); assert.deepStrictEqual( new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'a.js')))), new Set(['a']), @@ -199,14 +195,6 @@ parcel-transformer-b`, new Set(b.getUsedSymbols(nullthrows(findAsset(b, 'b.js')))), new Set(['b']), ); - assert.deepStrictEqual( - new Set(b.getUsedSymbols(findDependency(b, 'index.js', './a.js'))), - new Set(['a']), - ); - assert.deepStrictEqual( - new Set(b.getUsedSymbols(findDependency(b, 'index.js', './b.js'))), - new Set(['b']), - ); let calls = []; await run(b, { diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js index ab0927df6f6..857cfce2856 100644 --- a/packages/core/integration-tests/test/scope-hoisting.js +++ b/packages/core/integration-tests/test/scope-hoisting.js @@ -6,7 +6,6 @@ import {createWorkerFarm} from '@parcel/core'; import {md} from '@parcel/diagnostic'; import { assertBundles, - assertDependencyWasExcluded, bundle as _bundle, bundler as _bundler, distDir, @@ -1247,13 +1246,6 @@ describe('scope hoisting', function () { ), ); - assert.deepStrictEqual( - new Set( - b.getUsedSymbols(findDependency(b, 'a.js', './library/index.js')), - ), - new Set(['foo', 'foobar']), - ); - let calls = []; let output = await run(b, { sideEffect: v => { @@ -2237,6 +2229,39 @@ describe('scope hoisting', function () { assert.deepEqual(output.foo, 'bar'); }); + it('should correctly codesplit even with reexporting library index', async function () { + let b = await bundle( + path.join( + __dirname, + '/integration/scope-hoisting/es6/codesplit-reexports/src/entry.js', + ), + ); + + assertBundles(b, [ + { + type: 'js', + assets: [ + 'entry.js', + 'foo.js', + 'bar.js', + 'bundle-url.js', + 'cacheLoader.js', + 'js-loader.js', + ], + }, + { + type: 'js', + assets: ['async.js', 'foo2.js', 'bar2.js'], + }, + ]); + + let output = await run(b); + assert.deepEqual(output, [ + [20, 30], + [2, 3], + ]); + }); + it('should correctly handle circular dependencies', async function () { let b = await bundle( path.join(__dirname, '/integration/scope-hoisting/es6/circular/a.mjs'), @@ -2441,12 +2466,6 @@ describe('scope hoisting', function () { let output = await run(bundleEvent.bundleGraph); assert.deepEqual(output, [123]); - assertDependencyWasExcluded( - bundleEvent.bundleGraph, - 'a.js', - './c.js', - ); - await overlayFS.copyFile( path.join(testDir, 'index.2.js'), path.join(testDir, 'index.js'), @@ -2674,18 +2693,6 @@ describe('scope hoisting', function () { ), new Set(['gridSize']), ); - assert.deepStrictEqual( - new Set( - bundleEvent.bundleGraph.getUsedSymbols( - findDependency( - bundleEvent.bundleGraph, - 'theme.js', - './themeColors', - ), - ), - ), - new Set(), - ); assert(!findAsset(bundleEvent.bundleGraph, 'themeColors.js')); await overlayFS.copyFile( @@ -2713,18 +2720,8 @@ describe('scope hoisting', function () { ), new Set(['borderRadius', 'gridSize']), ); - assert.deepStrictEqual( - new Set( - bundleEvent.bundleGraph.getUsedSymbols( - findDependency( - bundleEvent.bundleGraph, - 'theme.js', - './themeColors', - ), - ), - ), - new Set('*'), - ); + assert(!findAsset(bundleEvent.bundleGraph, 'theme.js')); + assert(findAsset(bundleEvent.bundleGraph, 'themeColors.js')); await overlayFS.copyFile( path.join(testDir, 'index.1.js'), @@ -2748,18 +2745,7 @@ describe('scope hoisting', function () { ), new Set(['gridSize']), ); - assert.deepStrictEqual( - new Set( - bundleEvent.bundleGraph.getUsedSymbols( - findDependency( - bundleEvent.bundleGraph, - 'theme.js', - './themeColors', - ), - ), - ), - new Set(), - ); + assert(!findAsset(bundleEvent.bundleGraph, 'themeColors.js')); } finally { await subscription.unsubscribe(); } @@ -5490,7 +5476,7 @@ describe('scope hoisting', function () { shouldDisableCache: true, }); - await run(b); + assert.strictEqual(await run(b), 'bar'); await overlayFS.copyFile( path.join(testDir, 'index2.js'), @@ -5503,7 +5489,7 @@ describe('scope hoisting', function () { shouldDisableCache: false, }); - await run(b); + assert.strictEqual(await run(b), 'bar foo'); await overlayFS.copyFile( path.join(testDir, 'index3.js'), @@ -5516,8 +5502,7 @@ describe('scope hoisting', function () { shouldDisableCache: false, }); - let output = await run(b); - assert.strictEqual(output, 'bar foo bar'); + assert.strictEqual(await run(b), 'bar foo bar'); }); it("not insert unused requires that aren't registered anywhere", async function () { diff --git a/packages/core/test-utils/src/utils.js b/packages/core/test-utils/src/utils.js index 4642db29dd3..3fadfe85824 100644 --- a/packages/core/test-utils/src/utils.js +++ b/packages/core/test-utils/src/utils.js @@ -170,9 +170,15 @@ export function findDependency( `Couldn't find asset ${assetFileName}`, ); - let dependency = bundleGraph + let dependencies = bundleGraph .getDependencies(asset) - .find(d => d.specifier === specifier); + .filter(d => d.specifier === specifier); + + let dependency = + dependencies.length > 1 + ? dependencies.find(d => !bundleGraph.isDependencySkipped(d)) + : dependencies[0]; + invariant( dependency != null, `Couldn't find dependency ${assetFileName} -> ${specifier}`, diff --git a/packages/packagers/js/src/ScopeHoistingPackager.js b/packages/packagers/js/src/ScopeHoistingPackager.js index b990058b634..9e0d1755b39 100644 --- a/packages/packagers/js/src/ScopeHoistingPackager.js +++ b/packages/packagers/js/src/ScopeHoistingPackager.js @@ -9,6 +9,7 @@ import type { } from '@parcel/types'; import { + DefaultMap, PromiseQueue, relativeBundlePath, countLines, @@ -472,59 +473,64 @@ export class ScopeHoistingPackager { // If we matched an import, replace with the source code for the dependency. if (d != null) { - let dep = depMap.get(d); - if (!dep) { + let deps = depMap.get(d); + if (!deps) { return m; } - let resolved = this.bundleGraph.getResolvedAsset(dep, this.bundle); - let skipped = this.bundleGraph.isDependencySkipped(dep); - if (resolved && !skipped) { - // Hoist variable declarations for the referenced parcelRequire dependencies - // after the dependency is declared. This handles the case where the resulting asset - // is wrapped, but the dependency in this asset is not marked as wrapped. This means - // that it was imported/required at the top-level, so its side effects should run immediately. - let [res, lines] = this.getHoistedParcelRequires( - asset, - dep, - resolved, - ); - let map; - if ( - this.bundle.hasAsset(resolved) && - !this.seenAssets.has(resolved.id) - ) { - // If this asset is wrapped, we need to hoist the code for the dependency - // outside our parcelRequire.register wrapper. This is safe because all - // assets referenced by this asset will also be wrapped. Otherwise, inline the - // asset content where the import statement was. - if (shouldWrap) { - depContent.push(this.visitAsset(resolved)); - } else { - let [depCode, depMap, depLines] = this.visitAsset(resolved); - res = depCode + '\n' + res; - lines += 1 + depLines; - map = depMap; + let replacement = ''; + + // A single `${id}:${specifier}:esm` might have been resolved to multiple assets due to + // reexports. + for (let dep of deps) { + let resolved = this.bundleGraph.getResolvedAsset(dep, this.bundle); + let skipped = this.bundleGraph.isDependencySkipped(dep); + if (resolved && !skipped) { + // Hoist variable declarations for the referenced parcelRequire dependencies + // after the dependency is declared. This handles the case where the resulting asset + // is wrapped, but the dependency in this asset is not marked as wrapped. This means + // that it was imported/required at the top-level, so its side effects should run immediately. + let [res, lines] = this.getHoistedParcelRequires( + asset, + dep, + resolved, + ); + let map; + if ( + this.bundle.hasAsset(resolved) && + !this.seenAssets.has(resolved.id) + ) { + // If this asset is wrapped, we need to hoist the code for the dependency + // outside our parcelRequire.register wrapper. This is safe because all + // assets referenced by this asset will also be wrapped. Otherwise, inline the + // asset content where the import statement was. + if (shouldWrap) { + depContent.push(this.visitAsset(resolved)); + } else { + let [depCode, depMap, depLines] = this.visitAsset(resolved); + res = depCode + '\n' + res; + lines += 1 + depLines; + map = depMap; + } } - } - // Push this asset's source mappings down by the number of lines in the dependency - // plus the number of hoisted parcelRequires. Then insert the source map for the dependency. - if (sourceMap) { - if (lines > 0) { - sourceMap.offsetLines(lineCount + 1, lines); - } + // Push this asset's source mappings down by the number of lines in the dependency + // plus the number of hoisted parcelRequires. Then insert the source map for the dependency. + if (sourceMap) { + if (lines > 0) { + sourceMap.offsetLines(lineCount + 1, lines); + } - if (map) { - sourceMap.addSourceMap(map, lineCount); + if (map) { + sourceMap.addSourceMap(map, lineCount); + } } - } - lineCount += lines; - return res; + replacement += res; + lineCount += lines; + } } - - return ''; + return replacement; } // If it wasn't a dependency, then it was an inline replacement (e.g. $id$import$foo -> $id$export$foo). @@ -580,23 +586,24 @@ ${code} buildReplacements( asset: Asset, deps: Array, - ): [Map, Map] { + ): [Map>, Map] { let assetId = asset.meta.id; invariant(typeof assetId === 'string'); // Build two maps: one of import specifiers, and one of imported symbols to replace. // These will be used to build a regex below. - let depMap = new Map(); + let depMap = new DefaultMap>(() => []); let replacements = new Map(); for (let dep of deps) { let specifierType = dep.specifierType === 'esm' ? `:${dep.specifierType}` : ''; - depMap.set( - `${assetId}:${getSpecifier(dep)}${ - !dep.meta.placeholder ? specifierType : '' - }`, - dep, - ); + depMap + .get( + `${assetId}:${getSpecifier(dep)}${ + !dep.meta.placeholder ? specifierType : '' + }`, + ) + .push(dep); let asyncResolution = this.bundleGraph.resolveAsyncDependency( dep, diff --git a/packages/runtimes/service-worker/src/ServiceWorkerRuntime.js b/packages/runtimes/service-worker/src/ServiceWorkerRuntime.js index 90bee96c3d6..2aa01b0b72b 100644 --- a/packages/runtimes/service-worker/src/ServiceWorkerRuntime.js +++ b/packages/runtimes/service-worker/src/ServiceWorkerRuntime.js @@ -11,7 +11,8 @@ export default (new Runtime({ let asset = bundle.traverse((node, _, actions) => { if ( node.type === 'dependency' && - node.value.specifier === '@parcel/service-worker' + node.value.specifier === '@parcel/service-worker' && + !bundleGraph.isDependencySkipped(node.value) ) { actions.stop(); return bundleGraph.getResolvedAsset(node.value, bundle);