From 2dccfc517e98b00c5237d172b22cd3306ffc4293 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sat, 28 Sep 2024 09:37:48 -0700 Subject: [PATCH] Revert split manifest runtime bundles (#9955) --- packages/core/core/src/BundleGraph.js | 96 +---------------- packages/core/core/src/applyRuntimes.js | 65 +---------- .../core/integration-tests/test/bundler.js | 102 ------------------ packages/core/integration-tests/test/html.js | 30 ++++++ .../html-js-shared-head/package.json | 5 - .../integration/html-js-shared/package.json | 5 - .../integration/split-manifest-bundle/a.html | 1 - .../integration/split-manifest-bundle/a.js | 4 - .../integration/split-manifest-bundle/b.html | 1 - .../integration/split-manifest-bundle/b.js | 1 - .../integration/split-manifest-bundle/c.js | 1 - .../split-manifest-bundle/package.json | 6 -- .../split-manifest-bundle/yarn.lock | 0 .../core/integration-tests/test/workers.js | 5 +- packages/core/types-internal/src/index.js | 3 - packages/runtimes/js/src/JSRuntime.js | 76 +------------ 16 files changed, 37 insertions(+), 364 deletions(-) delete mode 100644 packages/core/integration-tests/test/integration/html-js-shared-head/package.json delete mode 100644 packages/core/integration-tests/test/integration/html-js-shared/package.json delete mode 100644 packages/core/integration-tests/test/integration/split-manifest-bundle/a.html delete mode 100644 packages/core/integration-tests/test/integration/split-manifest-bundle/a.js delete mode 100644 packages/core/integration-tests/test/integration/split-manifest-bundle/b.html delete mode 100644 packages/core/integration-tests/test/integration/split-manifest-bundle/b.js delete mode 100644 packages/core/integration-tests/test/integration/split-manifest-bundle/c.js delete mode 100644 packages/core/integration-tests/test/integration/split-manifest-bundle/package.json delete mode 100644 packages/core/integration-tests/test/integration/split-manifest-bundle/yarn.lock diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index e3916868ea2..22847dcf00c 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -5,7 +5,6 @@ import type { FilePath, Symbol, TraversalActions, - BundleBehavior as IBundleBehavior, } from '@parcel/types'; import type { ContentKey, @@ -20,10 +19,8 @@ import type { Bundle, BundleGraphNode, BundleGroup, - BundleNode, Dependency, DependencyNode, - Environment, InternalSourceLocation, Target, } from './types'; @@ -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 @@ -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( diff --git a/packages/core/core/src/applyRuntimes.js b/packages/core/core/src/applyRuntimes.js index cc5540a2493..b8090d3a41d 100644 --- a/packages/core/core/src/applyRuntimes.js +++ b/packages/core/core/src/applyRuntimes.js @@ -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({ bundleGraph, config, @@ -84,16 +64,7 @@ export default async function applyRuntimes({ let runtimes = await config.getRuntimes(); let connections: Array = []; - // 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; @@ -128,7 +99,6 @@ export default async function applyRuntimes({ filePath, isEntry, env, - priority, } of runtimeAssets) { let sourceName = path.join( path.dirname(filePath), @@ -144,36 +114,8 @@ export default async function applyRuntimes({ 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, @@ -192,9 +134,6 @@ export default async function applyRuntimes({ } } - // 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( diff --git a/packages/core/integration-tests/test/bundler.js b/packages/core/integration-tests/test/bundler.js index eb3cdc3ca2a..65c13fdf223 100644 --- a/packages/core/integration-tests/test/bundler.js +++ b/packages/core/integration-tests/test/bundler.js @@ -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 diff --git a/packages/core/integration-tests/test/html.js b/packages/core/integration-tests/test/html.js index 50eb7d1d2e2..bcd5ecf8295 100644 --- a/packages/core/integration-tests/test/html.js +++ b/packages/core/integration-tests/test/html.js @@ -11,6 +11,7 @@ import { outputFS, overlayFS, ncp, + fsFixture, } from '@parcel/test-utils'; import path from 'path'; @@ -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: + + + + + + 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); + }); }); diff --git a/packages/core/integration-tests/test/integration/html-js-shared-head/package.json b/packages/core/integration-tests/test/integration/html-js-shared-head/package.json deleted file mode 100644 index d7f640ea8dc..00000000000 --- a/packages/core/integration-tests/test/integration/html-js-shared-head/package.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "@parcel/runtime-js": { - "splitManifestThreshold": 99999999 - } -} \ No newline at end of file diff --git a/packages/core/integration-tests/test/integration/html-js-shared/package.json b/packages/core/integration-tests/test/integration/html-js-shared/package.json deleted file mode 100644 index d7f640ea8dc..00000000000 --- a/packages/core/integration-tests/test/integration/html-js-shared/package.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "@parcel/runtime-js": { - "splitManifestThreshold": 99999999 - } -} \ No newline at end of file diff --git a/packages/core/integration-tests/test/integration/split-manifest-bundle/a.html b/packages/core/integration-tests/test/integration/split-manifest-bundle/a.html deleted file mode 100644 index ee620ca3160..00000000000 --- a/packages/core/integration-tests/test/integration/split-manifest-bundle/a.html +++ /dev/null @@ -1 +0,0 @@ - diff --git a/packages/core/integration-tests/test/integration/split-manifest-bundle/a.js b/packages/core/integration-tests/test/integration/split-manifest-bundle/a.js deleted file mode 100644 index f18e990125a..00000000000 --- a/packages/core/integration-tests/test/integration/split-manifest-bundle/a.js +++ /dev/null @@ -1,4 +0,0 @@ - -import {c} from './b'; - -const ignore = () => import('./c'); \ No newline at end of file diff --git a/packages/core/integration-tests/test/integration/split-manifest-bundle/b.html b/packages/core/integration-tests/test/integration/split-manifest-bundle/b.html deleted file mode 100644 index e1c1a4b209c..00000000000 --- a/packages/core/integration-tests/test/integration/split-manifest-bundle/b.html +++ /dev/null @@ -1 +0,0 @@ - diff --git a/packages/core/integration-tests/test/integration/split-manifest-bundle/b.js b/packages/core/integration-tests/test/integration/split-manifest-bundle/b.js deleted file mode 100644 index f4938218b29..00000000000 --- a/packages/core/integration-tests/test/integration/split-manifest-bundle/b.js +++ /dev/null @@ -1 +0,0 @@ -export const c = () => import('./c'); \ No newline at end of file diff --git a/packages/core/integration-tests/test/integration/split-manifest-bundle/c.js b/packages/core/integration-tests/test/integration/split-manifest-bundle/c.js deleted file mode 100644 index 93c974371a5..00000000000 --- a/packages/core/integration-tests/test/integration/split-manifest-bundle/c.js +++ /dev/null @@ -1 +0,0 @@ -export const c = 'c'; diff --git a/packages/core/integration-tests/test/integration/split-manifest-bundle/package.json b/packages/core/integration-tests/test/integration/split-manifest-bundle/package.json deleted file mode 100644 index eb0c022e269..00000000000 --- a/packages/core/integration-tests/test/integration/split-manifest-bundle/package.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "@parcel/runtime-js": { - "splitManifestThreshold": 0 - } -} - \ No newline at end of file diff --git a/packages/core/integration-tests/test/integration/split-manifest-bundle/yarn.lock b/packages/core/integration-tests/test/integration/split-manifest-bundle/yarn.lock deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/packages/core/integration-tests/test/workers.js b/packages/core/integration-tests/test/workers.js index 8abaaffda6a..5cf8203adab 100644 --- a/packages/core/integration-tests/test/workers.js +++ b/packages/core/integration-tests/test/workers.js @@ -1231,11 +1231,10 @@ describe('parcel', function () { 'get-worker-url.js', 'lodash.js', 'esmodule-helpers.js', + 'bundle-manifest.js', + 'bundle-url.js', ], }, - { - assets: ['bundle-manifest.js', 'bundle-url.js'], - }, { assets: ['worker.js', 'lodash.js', 'esmodule-helpers.js'], }, diff --git a/packages/core/types-internal/src/index.js b/packages/core/types-internal/src/index.js index 5c123dcc190..769e4d8ab8d 100644 --- a/packages/core/types-internal/src/index.js +++ b/packages/core/types-internal/src/index.js @@ -1711,8 +1711,6 @@ export type Namer = {| |}): Async, |}; -type RuntimeAssetPriority = 'sync' | 'parallel'; - /** * A "synthetic" asset that will be inserted into the bundle graph. * @section runtime @@ -1723,7 +1721,6 @@ export type RuntimeAsset = {| +dependency?: Dependency, +isEntry?: boolean, +env?: EnvironmentOptions, - +priority?: RuntimeAssetPriority, |}; /** diff --git a/packages/runtimes/js/src/JSRuntime.js b/packages/runtimes/js/src/JSRuntime.js index f5058c1c542..47a85fdd8ff 100644 --- a/packages/runtimes/js/src/JSRuntime.js +++ b/packages/runtimes/js/src/JSRuntime.js @@ -11,12 +11,7 @@ import type { } from '@parcel/types'; import {Runtime} from '@parcel/plugin'; -import { - relativeBundlePath, - validateSchema, - type SchemaEntity, -} from '@parcel/utils'; -import {encodeJSONKeyComponent} from '@parcel/diagnostic'; +import {relativeBundlePath} from '@parcel/utils'; import path from 'path'; import nullthrows from 'nullthrows'; @@ -70,52 +65,8 @@ let bundleDependencies = new WeakMap< |}, >(); -type JSRuntimeConfig = {| - splitManifestThreshold: number, -|}; - -let defaultConfig: JSRuntimeConfig = { - splitManifestThreshold: 100000, -}; - -const CONFIG_SCHEMA: SchemaEntity = { - type: 'object', - properties: { - splitManifestThreshold: { - type: 'number', - }, - }, - additionalProperties: false, -}; - export default (new Runtime({ - async loadConfig({config, options}): Promise { - let packageKey = '@parcel/runtime-js'; - let conf = await config.getConfig([], { - packageKey, - }); - - if (!conf) { - return defaultConfig; - } - validateSchema.diagnostic( - CONFIG_SCHEMA, - { - data: conf?.contents, - source: await options.inputFS.readFile(conf.filePath, 'utf8'), - filePath: conf.filePath, - prependKey: `/${encodeJSONKeyComponent(packageKey)}`, - }, - packageKey, - `Invalid config for ${packageKey}`, - ); - - return { - ...defaultConfig, - ...conf?.contents, - }; - }, - apply({bundle, bundleGraph, options, config}) { + apply({bundle, bundleGraph, options}) { // Dependency ids in code replaced with referenced bundle names // Loader runtime added for bundle groups that don't have a native loader (e.g. HTML/CSS/Worker - isURL?), // and which are not loaded by a parent bundle. @@ -285,11 +236,6 @@ export default (new Runtime({ code: getRegisterCode(bundle, bundleGraph), isEntry: true, env: {sourceType: 'module'}, - priority: getManifestBundlePriority( - bundleGraph, - bundle, - config.splitManifestThreshold, - ), }); } @@ -722,21 +668,3 @@ function shouldUseRuntimeManifest( options.mode === 'production' ); } - -function getManifestBundlePriority( - bundleGraph: BundleGraph, - bundle: NamedBundle, - threshold: number, -): $PropertyType { - let bundleSize = 0; - - bundle.traverseAssets((asset, _, actions) => { - bundleSize += asset.stats.size; - - if (bundleSize > threshold) { - actions.stop(); - } - }); - - return bundleSize > threshold ? 'parallel' : 'sync'; -}