-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix missing module in large app from Experimental Bundler (#8303)
* 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 <[email protected]>
- Loading branch information
Showing
45 changed files
with
513 additions
and
193 deletions.
There are no files selected for viewing
240 changes: 136 additions & 104 deletions
240
packages/bundlers/experimental/src/ExperimentalBundler.js
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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
3
.../core/integration-tests/test/integration/shared-bundle-between-reused-bundle-removal/a.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import foo from './foo'; | ||
|
||
export default 'a'; |
2 changes: 2 additions & 0 deletions
2
.../core/integration-tests/test/integration/shared-bundle-between-reused-bundle-removal/b.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
|
||
export default 5; |
6 changes: 6 additions & 0 deletions
6
...ore/integration-tests/test/integration/shared-bundle-between-reused-bundle-removal/bar.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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
1
...re/integration-tests/test/integration/shared-bundle-between-reused-bundle-removal/buzz.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
import c from './c'; |
Oops, something went wrong.