Skip to content

Commit

Permalink
Revert split manifest runtime bundles (#9955)
Browse files Browse the repository at this point in the history
  • Loading branch information
devongovett authored Sep 28, 2024
1 parent 29441fd commit 2dccfc5
Show file tree
Hide file tree
Showing 16 changed files with 37 additions and 364 deletions.
96 changes: 1 addition & 95 deletions packages/core/core/src/BundleGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type {
FilePath,
Symbol,
TraversalActions,
BundleBehavior as IBundleBehavior,
} from '@parcel/types';
import type {
ContentKey,
Expand All @@ -20,10 +19,8 @@ import type {
Bundle,
BundleGraphNode,
BundleGroup,
BundleNode,
Dependency,
DependencyNode,
Environment,
InternalSourceLocation,
Target,
} from './types';
Expand All @@ -40,8 +37,7 @@ import {DefaultMap, objectSortedEntriesDeep, getRootDir} from '@parcel/utils';
import {Priority, BundleBehavior, SpecifierType} from './types';
import {getBundleGroupId, getPublicId} from './utils';
import {ISOLATED_ENVS} from './public/Environment';
import {fromProjectPath, fromProjectPathRelative} from './projectPath';
import {HASH_REF_PREFIX} from './constants';
import {fromProjectPath} from './projectPath';

export const bundleGraphEdgeTypes = {
// A lack of an edge type indicates to follow the edge while traversing
Expand Down Expand Up @@ -463,96 +459,6 @@ export default class BundleGraph {
});
}

createBundle(
opts:
| {|
+entryAsset: Asset,
+target: Target,
+needsStableName?: ?boolean,
+bundleBehavior?: ?IBundleBehavior,
+shouldContentHash: boolean,
+env: Environment,
|}
| {|
+type: string,
+env: Environment,
+uniqueKey: string,
+target: Target,
+needsStableName?: ?boolean,
+bundleBehavior?: ?IBundleBehavior,
+isSplittable?: ?boolean,
+pipeline?: ?string,
+shouldContentHash: boolean,
|},
): Bundle {
let {entryAsset, target} = opts;
let bundleId = hashString(
'bundle:' +
(opts.entryAsset ? opts.entryAsset.id : opts.uniqueKey) +
fromProjectPathRelative(target.distDir) +
(opts.bundleBehavior ?? ''),
);

let existing = this._graph.getNodeByContentKey(bundleId);
if (existing != null) {
invariant(existing.type === 'bundle');
return existing.value;
}

let publicId = getPublicId(bundleId, existing =>
this._bundlePublicIds.has(existing),
);
this._bundlePublicIds.add(publicId);

let isPlaceholder = false;
if (entryAsset) {
let entryAssetNode = this._graph.getNodeByContentKey(entryAsset.id);
invariant(entryAssetNode?.type === 'asset', 'Entry asset does not exist');
isPlaceholder = entryAssetNode.requested === false;
}

let bundleNode: BundleNode = {
type: 'bundle',
id: bundleId,
value: {
id: bundleId,
hashReference: opts.shouldContentHash
? HASH_REF_PREFIX + bundleId
: bundleId.slice(-8),
type: opts.entryAsset ? opts.entryAsset.type : opts.type,
env: opts.env,
entryAssetIds: entryAsset ? [entryAsset.id] : [],
mainEntryId: entryAsset?.id,
pipeline: opts.entryAsset ? opts.entryAsset.pipeline : opts.pipeline,
needsStableName: opts.needsStableName,
bundleBehavior:
opts.bundleBehavior != null
? BundleBehavior[opts.bundleBehavior]
: null,
isSplittable: opts.entryAsset
? opts.entryAsset.isBundleSplittable
: opts.isSplittable,
isPlaceholder,
target,
name: null,
displayName: null,
publicId,
},
};

let bundleNodeId = this._graph.addNodeByContentKey(bundleId, bundleNode);

if (opts.entryAsset) {
this._graph.addEdge(
bundleNodeId,
this._graph.getNodeIdByContentKey(opts.entryAsset.id),
);
}

invariant;
return bundleNode.value;
}

addAssetToBundle(asset: Asset, bundle: Bundle) {
let bundleNodeId = this._graph.getNodeIdByContentKey(bundle.id);
this._graph.addEdge(
Expand Down
65 changes: 2 additions & 63 deletions packages/core/core/src/applyRuntimes.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,6 @@ type RuntimeConnection = {|
isEntry: ?boolean,
|};

function nameRuntimeBundle(
bundle: InternalBundle,
siblingBundle: InternalBundle,
) {
// We don't run custom namers on runtime bundles as the runtime assumes that they are
// located at the same nesting level as their owning bundle. Custom naming could
// be added in future as long as the custom name is validated.
let {hashReference} = bundle;

let name = nullthrows(siblingBundle.name)
// Remove the existing hash from standard file patterns
// e.g. 'main.[hash].js' -> 'main.js' or 'main~[hash].js' -> 'main.js'
.replace(new RegExp(`[\\.~\\-_]?${siblingBundle.hashReference}`), '')
// Ensure the file ends with 'runtime.[hash].js'
.replace(`.${bundle.type}`, `.runtime.${hashReference}.${bundle.type}`);

bundle.name = name;
bundle.displayName = name.replace(hashReference, '[hash]');
}

export default async function applyRuntimes<TResult: RequestResult>({
bundleGraph,
config,
Expand All @@ -84,16 +64,7 @@ export default async function applyRuntimes<TResult: RequestResult>({
let runtimes = await config.getRuntimes();
let connections: Array<RuntimeConnection> = [];

// As manifest bundles may be added during runtimes we process them in reverse topological
// sort order. This allows bundles to be added to their bundle groups before they are referenced
// by other bundle groups by loader runtimes
let bundles = [];
bundleGraph.traverseBundles({
exit(bundle) {
bundles.push(bundle);
},
});

let bundles = bundleGraph.getBundles({includeInline: true});
for (let bundle of bundles) {
for (let runtime of runtimes) {
let measurement;
Expand Down Expand Up @@ -128,7 +99,6 @@ export default async function applyRuntimes<TResult: RequestResult>({
filePath,
isEntry,
env,
priority,
} of runtimeAssets) {
let sourceName = path.join(
path.dirname(filePath),
Expand All @@ -144,36 +114,8 @@ export default async function applyRuntimes<TResult: RequestResult>({
isSource: true,
};

let connectionBundle = bundle;

if (priority === 'parallel' && !bundle.needsStableName) {
let bundleGroups =
bundleGraph.getBundleGroupsContainingBundle(bundle);

connectionBundle = nullthrows(
bundleGraph.createBundle({
type: bundle.type,
needsStableName: false,
env: bundle.env,
target: bundle.target,
uniqueKey: 'runtime-manifest:' + bundle.id,
shouldContentHash: options.shouldContentHash,
}),
);

for (let bundleGroup of bundleGroups) {
bundleGraph.addBundleToBundleGroup(
connectionBundle,
bundleGroup,
);
}
bundleGraph.createBundleReference(bundle, connectionBundle);

nameRuntimeBundle(connectionBundle, bundle);
}

connections.push({
bundle: connectionBundle,
bundle,
assetGroup,
dependency,
isEntry,
Expand All @@ -192,9 +134,6 @@ export default async function applyRuntimes<TResult: RequestResult>({
}
}

// Correct connection order after generating runtimes in reverse order
connections.reverse();

// Add dev deps for runtime plugins AFTER running them, to account for lazy require().
for (let runtime of runtimes) {
let devDepRequest = await createDevDependency(
Expand Down
102 changes: 0 additions & 102 deletions packages/core/integration-tests/test/bundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -844,108 +844,6 @@ describe('bundler', function () {
);
});

it('should split manifest bundle', async function () {
let b = await bundle(
[
path.join(__dirname, 'integration/split-manifest-bundle/a.html'),
path.join(__dirname, 'integration/split-manifest-bundle/b.html'),
],
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
shouldOptimize: false,
},
},
);

// There should be two manifest bundles added, one for a.js, one for b.js
assertBundles(b, [
{
assets: ['a.html'],
},
{
assets: ['b.html'],
},
{
assets: ['a.js', 'cacheLoader.js', 'js-loader.js'],
},
{
assets: ['bundle-manifest.js', 'bundle-url.js'], // manifest bundle
},
{
assets: [
'b.js',
'cacheLoader.js',
'js-loader.js',
'esmodule-helpers.js',
],
},
{
assets: ['bundle-manifest.js', 'bundle-url.js'], // manifest bundle
},
{
assets: ['c.js'],
},
]);

let aManifestBundle = b
.getBundles()
.find(
bundle => !bundle.getMainEntry() && bundle.name.includes('runtime'),
);

let bBundles = b
.getBundles()
.filter(bundle => /b\.HASH_REF/.test(bundle.name));

let aBundleManifestAsset;
aManifestBundle.traverseAssets((asset, _, {stop}) => {
if (/runtime-[a-z0-9]{16}\.js/.test(asset.filePath)) {
aBundleManifestAsset = asset;
stop();
}
});
let aBundleManifestAssetCode = await aBundleManifestAsset.getCode();

// Assert the a.js manifest bundle is aware of all the b.js bundles
for (let bundle of bBundles) {
assert(
aBundleManifestAssetCode.includes(bundle.name),
`Bundle should contain reference to: "${bundle.name}"`,
);
}
});

it('should not split manifest bundle for stable entries', async function () {
let b = await bundle(
path.join(__dirname, 'integration/split-manifest-bundle/a.js'),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);

assertBundles(b, [
{
assets: [
'a.js',
'b.js',
'bundle-url.js',
'cacheLoader.js',
'js-loader.js',
'esmodule-helpers.js',
'bundle-manifest.js',
],
},
{
assets: ['c.js'],
},
]);
});

it('should respect mode specific config', async function () {
await fsFixture(overlayFS, __dirname)`
mode-specific-bundler-config
Expand Down
30 changes: 30 additions & 0 deletions packages/core/integration-tests/test/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
outputFS,
overlayFS,
ncp,
fsFixture,
} from '@parcel/test-utils';
import path from 'path';

Expand Down Expand Up @@ -3055,4 +3056,33 @@ describe('html', function () {

await run(b, {output: null}, {require: false});
});

it('should insert bundle manifest into the correct bundle with multiple script tags', async function () {
const dir = path.join(__dirname, 'manifest-multi-script');
overlayFS.mkdirp(dir);

await fsFixture(overlayFS, dir)`
index.html:
<body>
<script src="./polyfills.js" type="module"></script>
<script src="./main.js" type="module"></script>
</body>
polyfills.js:
import('./polyfills-async');
polyfills-async.js:
export const foo = 2;
main.js:
import('./main-async');
main-async.js:
export const bar = 3;
`;

let b = await bundle(path.join(dir, '/index.html'), {
inputFS: overlayFS,
});

// Should not error with "Cannot find module" error at runtime.
await run(b);
});
});

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading

0 comments on commit 2dccfc5

Please sign in to comment.