Skip to content

Commit

Permalink
Fix missing module in large app from Experimental Bundler (#8303)
Browse files Browse the repository at this point in the history 
* consider reused bundles as shared bundles, turn source bundles into set, add tests for removal based on limit and size

* do not consider reuse bundles for min size removal as it breaks build

* rename reusing of bundles step and de fork test

* Handle case where rused bundle pulls in shared and then gets removed under parallel request limit

* add to source bundles for shared bundle

* disable test for default due to varied functionality

* break out of sourceBundle iteration after a bundle is removed

Co-authored-by: Gora Kong <gora.kong1@gmail.com>
  • Loading branch information
@AGawrys @gorakong
AGawrys and gorakong committed Jul 27, 2022
1 parent ad5b777 commit 70b2abc
Show file tree
Hide file tree
Showing 45 changed files with 513 additions and 193 deletions.
240 changes: 136 additions & 104 deletions packages/bundlers/experimental/src/ExperimentalBundler.js
View file

Large diffs are not rendered by default.

280 changes: 194 additions & 86 deletions packages/core/integration-tests/test/bundler.js
View file
@@ -1,109 +1,217 @@
import assert from 'assert';
import sinon from 'sinon';
import path from 'path';
import {
assertBundleTree,
bundle,
bundler,
nextBundle,
} from '@parcel/test-utils';

describe.skip('bundler', function () {
it('should bundle once before exporting middleware', async function () {
let b = bundler(
path.join(__dirname, '/integration/bundler-middleware/index.js'),
);
b.middleware();

await nextBundle(b);
assert(b.entryAssets);
});

it('should defer bundling if a bundle is pending', async () => {
const b = bundler(path.join(__dirname, '/integration/html/index.html'));
b.pending = true; // bundle in progress
const spy = sinon.spy(b, 'bundle');

// first bundle, with existing bundle pending
const bundlePromise = b.bundle();

// simulate bundle finished
b.pending = false;
b.emit('buildEnd');

// wait for bundle to complete
await bundlePromise;

assert(spy.calledTwice);
});

it('should enforce asset type path to be a string', () => {
const b = bundler(path.join(__dirname, '/integration/html/index.html'));

assert.throws(() => {
b.addAssetType('.ext', {});
}, 'should be a module path');
});

it('should enforce setup before bundling', () => {
const b = bundler(path.join(__dirname, '/integration/html/index.html'));
b.farm = true; // truthy

assert.throws(() => {
b.addAssetType('.ext', __filename);
}, 'before bundling');

assert.throws(() => {
b.addPackager('type', 'packager');
}, 'before bundling');
import assert from 'assert';
import {bundle, assertBundles, findAsset} from '@parcel/test-utils';

describe('bundler', function () {
it('should remove reused bundle (over shared bundles based on size) if the bundlegroup hit the parallel request limit', async function () {
if (process.env.PARCEL_TEST_EXPERIMENTAL_BUNDLER) {
let b = await bundle(
path.join(
__dirname,
'integration/shared-bundle-reused-bundle-remove-reuse/index.js',
),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);

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

it('should support multiple entry points', async function () {
let b = await bundle([
path.join(__dirname, '/integration/multi-entry/one.html'),
path.join(__dirname, '/integration/multi-entry/two.html'),
]);
//This test case is the sdame as previous except we remove the shared bundle since it is smaller
it('should remove shared bundle (over reused bundles based on size) if the bundlegroup hit the parallel request limit', async function () {
let b = await bundle(
path.join(
__dirname,
'integration/shared-bundle-reused-bundle-remove-shared/index.js',
),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);

await assertBundleTree(b, [
assertBundles(b, [
{
type: 'html',
assets: ['one.html'],
childBundles: [
{
type: 'js',
assets: ['shared.js'],
},
name: 'index.js',
assets: [
'index.js',
'bundle-url.js',
'cacheLoader.js',
'css-loader.js',
'esmodule-helpers.js',
'js-loader.js',
'bundle-manifest.js',
],
},
{
type: 'html',
assets: ['two.html'],
childBundles: [],
assets: ['bar.js', 'c.js'],
},
{
// A consequence of our shared bundle 'c' being removed for the bundleGroup bar
// is that it must also be removed for buzz, even though the buzz bundleGroup does not
// hit the parallel request limit. This is because the shared bundle is no longer sharing
// it is only attached to one bundle and thus should be removed.
assets: ['buzz.js', 'c.js'],
},
{
assets: ['a.js', 'b.js', 'foo.js'],
},
{
assets: ['styles.css'],
},
{
assets: ['local.html'],
},
]);
});

it('should support multiple entry points as a glob', async function () {
it('should not remove shared bundle from graph if one bundlegroup hits the parallel request limit, and at least 2 other bundleGroups that need it do not', async function () {
//The shared bundle should only be 'put back' for the bundlegroups which hit the parallel request limit
// But if there are at least two other bundlegroups using this shared bundle that do not hit the max limit
// the shared bundle should not be removed from the graph
let b = await bundle(
path.join(__dirname, '/integration/multi-entry/*.html'),
path.join(
__dirname,
'integration/shared-bundle-remove-from-one-group-only/index.js',
),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);

await assertBundleTree(b, [
assertBundles(b, [
{
type: 'html',
assets: ['one.html'],
childBundles: [
{
type: 'js',
assets: ['shared.js'],
},
name: 'index.js',
assets: [
'index.js',
'bundle-url.js',
'cacheLoader.js',
'css-loader.js',
'esmodule-helpers.js',
'js-loader.js',
'bundle-manifest.js',
],
},
{
type: 'html',
assets: ['two.html'],
childBundles: [],
assets: ['bar.js', 'c.js'], // shared bundle merged back
},
{
assets: ['buzz.js'],
},
{
assets: ['c.js'], // shared bundle
},
{
assets: ['foo.js'],
},
{
assets: ['styles.css'],
},
{
assets: ['local.html'],
},
]);
});

it('should not remove shared bundle from graph if its parent (a reused bundle) is removed by parallel request limit', async function () {
//The shared bundle should only be 'put back' for the bundlegroups which hit the parallel request limit
// But if there are at least two other bundlegroups using this shared bundle that do not hit the max limit
// the shared bundle should not be removed from the graph
if (process.env.PARCEL_TEST_EXPERIMENTAL_BUNDLER) {
let b = await bundle(
path.join(
__dirname,
'integration/shared-bundle-between-reused-bundle-removal/index.js',
),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);

assertBundles(b, [
{
name: 'index.js',
assets: [
'index.js',
'bundle-url.js',
'cacheLoader.js',
'css-loader.js',
'esmodule-helpers.js',
'js-loader.js',
'bundle-manifest.js',
],
},
{
assets: ['bar.js', 'foo.js', 'a.js', 'b.js'], // shared bundle merged back
},
{
assets: ['buzz.js'],
},
{
assets: ['c.js'], // shared bundle
},
{
assets: ['foo.js', 'a.js', 'b.js'],
},
{
assets: ['styles.css'],
},
{
assets: ['local.html'],
},
]);

assert(
b
.getReferencedBundles(
b.getBundlesWithAsset(findAsset(b, 'bar.js'))[0],
)
.includes(b.getBundlesWithAsset(findAsset(b, 'c.js'))[0]),
);
}
});
});
3 changes: 3 additions & 0 deletions .../core/integration-tests/test/integration/shared-bundle-between-reused-bundle-removal/a.js
View file
@@ -0,0 +1,3 @@
import foo from './foo';

export default 'a';
2 changes: 2 additions & 0 deletions .../core/integration-tests/test/integration/shared-bundle-between-reused-bundle-removal/b.js
View file
@@ -0,0 +1,2 @@

export default 5;
6 changes: 6 additions & 0 deletions ...ore/integration-tests/test/integration/shared-bundle-between-reused-bundle-removal/bar.js
View file
@@ -0,0 +1,6 @@
import foo from './a';
import bar from './b';
import styles from './styles.css';
import html from './local.html';

export default 4;
1 change: 1 addition & 0 deletions ...re/integration-tests/test/integration/shared-bundle-between-reused-bundle-removal/buzz.js
View file
@@ -0,0 +1 @@
import c from './c';

0 comments on commit 70b2abc

Please sign in to comment.