Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

achan3/P2X-1041 new disableSharedBundles config option #9209

Merged
merged 24 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0196070
Add initial config logic to disable shared bundles
irismoini Aug 21, 2023
eb7147b
Add test cases for disableSharedBundles config
irismoini Aug 21, 2023
7713bed
Shared bundle creation now relies on disableSharedBundles
irismoini Aug 21, 2023
cbce809
Add yarn.lock
irismoini Aug 21, 2023
fd2cdaa
add new logger warning when both minBundleSize and disableSharedBundl…
Aug 22, 2023
389bf2a
Revert "add new logger warning when both minBundleSize and disableSha…
Aug 22, 2023
b3039b6
add warning for conflicting setting of both minBundleSize and disable…
Aug 22, 2023
84468bf
add test when disableSharedBundler config option isn't set - revert t…
Aug 22, 2023
d4b49ec
Merge branch 'v2' into achan3/P2X-1041-add-warnings
Aug 23, 2023
a8eebf0
add test cases for instances that should not throw warnings
Aug 23, 2023
619ae37
add test case where warning is expected
Aug 23, 2023
637f28f
merge with latest v2
Aug 24, 2023
ab27d62
edit defaultBundler to throw warnings for minBundles and maxParallelR…
Aug 25, 2023
4ce2090
condense test cases into minimal combos to test minBundles and maxPar…
Aug 25, 2023
24fe690
Merge branch 'v2' into achan3/P2X-1041-add-warnings
Aug 28, 2023
18f6510
prevent reuse of bundles when disableSharedBundles is set
Aug 29, 2023
160a529
remove import used only for testing
Aug 29, 2023
0d69d91
Merge branch 'v2' into achan3/P2X-1041-add-warnings
Sep 8, 2023
965916e
add warning for minBundleSize when shared bundles are disabled
Sep 8, 2023
20163ec
add warning for minBundleSize, remove duplicate disableSharedConfig c…
Sep 8, 2023
35bb7ad
stop removal of shared bundles if shared bundles are disabled to begi…
Sep 8, 2023
a625015
Merge branch 'v2' into achan3/P2X-1041-add-warnings
Sep 11, 2023
2e79912
Merge branch 'v2' into achan3/P2X-1041-add-warnings
adelchan07 Sep 19, 2023
31a0bb3
Merge branch 'v2' into achan3/P2X-1041-add-warnings
adelchan07 Sep 21, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions packages/bundlers/default/src/DefaultBundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {ContentGraph, Graph} from '@parcel/graph';
import invariant from 'assert';
import {ALL_EDGE_TYPES} from '@parcel/graph';
import {Bundler} from '@parcel/plugin';
import logger from '@parcel/logger';
import {setEqual, validateSchema, DefaultMap, BitSet} from '@parcel/utils';
import nullthrows from 'nullthrows';
import {encodeJSONKeyComponent} from '@parcel/diagnostic';
Expand All @@ -28,12 +29,14 @@ type BundlerConfig = {|
minBundles?: number,
minBundleSize?: number,
maxParallelRequests?: number,
disableSharedBundles?: boolean,
|};

type ResolvedBundlerConfig = {|
minBundles: number,
minBundleSize: number,
maxParallelRequests: number,
disableSharedBundles: boolean,
|};

// Default options by http version.
Expand All @@ -42,11 +45,13 @@ const HTTP_OPTIONS = {
minBundles: 1,
minBundleSize: 30000,
maxParallelRequests: 6,
disableSharedBundles: false,
},
'2': {
minBundles: 1,
minBundleSize: 20000,
maxParallelRequests: 25,
disableSharedBundles: false,
},
};

Expand Down Expand Up @@ -958,8 +963,10 @@ function createIdealGraph(
entryBundle.size += asset.stats.size;
}

// Create shared bundles for splittable bundles.
if (reachable.length > config.minBundles) {
if (
config.disableSharedBundles === false &&
reachable.length > config.minBundles
) {
let sourceBundles = reachable.map(a => nullthrows(bundles.get(a.id)));
let key = reachable.map(a => a.id).join(',');
let bundleId = bundles.get(key);
Expand Down Expand Up @@ -1010,7 +1017,10 @@ function createIdealGraph(
value: bundle,
type: 'bundle',
});
} else if (reachable.length <= config.minBundles) {
} else if (
config.disableSharedBundles === true ||
reachable.length <= config.minBundles
) {
for (let root of reachable) {
let bundle = nullthrows(
bundleGraph.getNode(nullthrows(bundleRoots.get(root))[0]),
Expand Down Expand Up @@ -1277,6 +1287,9 @@ const CONFIG_SCHEMA: SchemaEntity = {
maxParallelRequests: {
type: 'number',
},
disableSharedBundles: {
type: 'boolean',
},
},
additionalProperties: false,
};
Expand Down Expand Up @@ -1357,6 +1370,17 @@ async function loadBundlerConfig(

invariant(conf?.contents != null);

// minBundles value will be ignored if shared bundles are disabled
adelchan07 marked this conversation as resolved.
Show resolved Hide resolved
if (
conf.contents.minBundleSize != null &&
adelchan07 marked this conversation as resolved.
Show resolved Hide resolved
conf.contents.disableSharedBundles === true
) {
logger.warn({
origin: '@parcel/bundler-default',
message: `The value of "${conf.contents.minBundleSize}" set for minBundleSize will not be used as shared bundles have been disabled`,
});
}
adelchan07 marked this conversation as resolved.
Show resolved Hide resolved

validateSchema.diagnostic(
CONFIG_SCHEMA,
{
Expand All @@ -1377,6 +1401,8 @@ async function loadBundlerConfig(
minBundleSize: conf.contents.minBundleSize ?? defaults.minBundleSize,
maxParallelRequests:
conf.contents.maxParallelRequests ?? defaults.maxParallelRequests,
disableSharedBundles:
conf.contents.disableSharedBundles ?? defaults.disableSharedBundles,
};
}

Expand Down
248 changes: 248 additions & 0 deletions packages/core/integration-tests/test/bundler.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,256 @@
import path from 'path';
import assert from 'assert';
import Logger from '@parcel/logger';
import {bundle, assertBundles, findAsset} from '@parcel/test-utils';

describe('bundler', function () {
it('should throw warnings when disabled shared bundles and a min bundle size value is given', async function () {
adelchan07 marked this conversation as resolved.
Show resolved Hide resolved
let messages = [];
let loggerDisposable = Logger.onLog(message => {
messages.push(message);
});

await bundle(
path.join(
__dirname,
'integration/disable-shared-bundles-with-min-bundle-size/index.js',
),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);
loggerDisposable.dispose();

assert.deepEqual(messages, [
{
type: 'log',
level: 'warn',
diagnostics: [
{
origin: '@parcel/bundler-default',
message:
'The value of "2000" set for minBundleSize will not be used as shared bundles have been disabled',
},
],
},
]);
});

it('should not throw warnings when shared bundles are enabled and a min bundle size value is given', async function () {
let messages = [];
let loggerDisposable = Logger.onLog(message => {
messages.push(message);
});

await bundle(
path.join(
__dirname,
'integration/disable-shared-bundles-false-with-min-bundle-size/index.js',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since disable-shared-bundles-false-with-min-bundle-size has essentially the same contents as disable-shared-bundles-false we could probably reuse one of them, and get rid of the other

),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);
loggerDisposable.dispose();

assert.deepEqual(messages, []);
});

it('should not throw warnings when disableSharedBundles is not set and a min bundle size value is given', async function () {
let messages = [];
let loggerDisposable = Logger.onLog(message => {
messages.push(message);
});

await bundle(
path.join(
__dirname,
'integration/no-disable-shared-bundles-with-min-bundle-size/index.js',
),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);
loggerDisposable.dispose();

assert.deepEqual(messages, []);
});

it('should not throw warnings when shared bundles are disabled but min bundle size is not set', async function () {
let messages = [];
let loggerDisposable = Logger.onLog(message => {
messages.push(message);
});

await bundle(
path.join(
__dirname,
'integration/disable-shared-bundles-no-min-bundle-size/index.js',
),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);
loggerDisposable.dispose();

assert.deepEqual(messages, []);
});

it('should create shared bundles when disableSharedBundles is not set', async function () {
let b = await bundle(
path.join(
__dirname,
'integration/disable-shared-bundles-default/index.js',
),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);

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

it('should create shared bundles when disableSharedBundles is not set', async function () {
let b = await bundle(
path.join(
__dirname,
'integration/disable-shared-bundles-default/index.js',
),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);

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

it('should not create shared bundles when disableSharedBundles is set to true', async function () {
let b = await bundle(
path.join(__dirname, 'integration/disable-shared-bundles-true/index.js'),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);

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

it('should create shared bundles when disableSharedBundles is set to false', async function () {
let b = await bundle(
path.join(__dirname, 'integration/disable-shared-bundles-false/index.js'),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);

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

it('should not count inline assests towards parallel request limit', async function () {
// Shared bundle should not be removed in this case
let b = await bundle(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 5;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 4;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import a from './a';
import b from './b';

export default 3;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import a from './a';
import b from './b';

export default 2;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import('./foo');
import('./bar');

export default 1;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"@parcel/bundler-default": {
"minBundles": 0,
"minBundleSize": 200,
"maxParallelRequests": 100
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 5;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 4;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import a from './a';
import b from './b';

export default 3;
Loading
Loading