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

Support parallel bundle imports in libraries #9156

Merged
merged 2 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions packages/bundlers/default/src/DefaultBundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ function createIdealGraph(
*/
let bundleGroupRootAsset = nullthrows(bundleGroup.mainEntryAsset);
if (
parentAsset.type !== childAsset.type &&
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

entries.has(bundleGroupRootAsset) &&
canMerge(bundleGroupRootAsset, childAsset) &&
dependency.bundleBehavior == null
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"extends": "@parcel/config-default",
"resolvers": ["./ParallelResolver", "..."]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const {Resolver} = require('@parcel/plugin');

module.exports = new Resolver({
resolve({specifier}) {
if (specifier === './foo') {
return {
filePath: __dirname + '/foo.js',
priority: 'parallel'
};
}
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'foo';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as foo from './foo';

export default foo + ' bar';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"module": "dist/out.js"
}
28 changes: 28 additions & 0 deletions packages/core/integration-tests/test/output-formats.js
Original file line number Diff line number Diff line change
Expand Up @@ -1463,6 +1463,34 @@ describe('output formats', function () {
assert.deepEqual(calls, [[['a', 10]]]);
});

it('should support external parallel dependencies', async function () {
let b = await bundle(
path.join(__dirname, 'integration/library-parallel-deps/index.js'),
{
mode: 'production',
defaultTargetOptions: {
shouldOptimize: false,
},
},
);

assertBundles(b, [
{
name: 'out.js',
assets: ['index.js'],
},
{
assets: ['foo.js'],
},
]);

let res = await run(b);
assert.equal(res.default, 'foo bar');

let content = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
assert(/import [a-z0-9$]+ from "\.\//.test(content));
});

describe('global', function () {
it.skip('should support split bundles between main script and workers', async function () {
let b = await bundle(
Expand Down
49 changes: 38 additions & 11 deletions packages/packagers/js/src/ScopeHoistingPackager.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export class ScopeHoistingPackager {
hoistedRequires: Map<string, Map<string, string>> = new Map();
needsPrelude: boolean = false;
usedHelpers: Set<string> = new Set();
externalAssets: Set<Asset> = new Set();

constructor(
options: PluginOptions,
Expand Down Expand Up @@ -130,9 +131,25 @@ export class ScopeHoistingPackager {
this.bundle.env.isLibrary ||
this.bundle.env.outputFormat === 'commonjs'
) {
let bundles = this.bundleGraph.getReferencedBundles(this.bundle);
for (let b of bundles) {
this.externals.set(relativeBundlePath(this.bundle, b), new Map());
for (let b of this.bundleGraph.getReferencedBundles(this.bundle)) {
let entry = b.getMainEntry();
let symbols = new Map();
if (entry && !this.isAsyncBundle && entry.type === 'js') {
Copy link
Member Author

Choose a reason for hiding this comment

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

for now async bundles in libraries are still referenced with parcelRequire. Changing that would require additional changes to the runtimes, but we could fix it eventually.

this.externalAssets.add(entry);

let usedSymbols = this.bundleGraph.getUsedSymbols(entry) || new Set();
for (let s of usedSymbols) {
// If the referenced bundle is ESM, and we are importing '*', use 'default' instead.
// This matches the logic below in buildExportedSymbols.
let imported = s;
if (imported === '*' && b.env.outputFormat === 'esmodule') {
imported = 'default';
}
symbols.set(imported, this.getSymbolResolution(entry, entry, s));
}
}

this.externals.set(relativeBundlePath(this.bundle, b), symbols);
}
}

Expand Down Expand Up @@ -756,6 +773,13 @@ ${code}
}
}

isWrapped(resolved: Asset, parentAsset: Asset): boolean {
return (
(!this.bundle.hasAsset(resolved) && !this.externalAssets.has(resolved)) ||
(this.wrappedAssets.has(resolved.id) && resolved !== parentAsset)
);
}

getSymbolResolution(
parentAsset: Asset,
resolved: Asset,
Expand All @@ -777,13 +801,18 @@ ${code}
return '{}';
}

let isWrapped =
!this.bundle.hasAsset(resolvedAsset) ||
(this.wrappedAssets.has(resolvedAsset.id) &&
resolvedAsset !== parentAsset);
let isWrapped = this.isWrapped(resolvedAsset, parentAsset);
let staticExports = resolvedAsset.meta.staticExports !== false;
let publicId = this.bundleGraph.getAssetPublicId(resolvedAsset);

// External CommonJS dependencies need to be accessed as an object property rather than imported
// directly to maintain live binding.
let isExternalCommonJS =
!isWrapped &&
this.bundle.env.isLibrary &&
this.bundle.env.outputFormat === 'commonjs' &&
!this.bundle.hasAsset(resolvedAsset);

// If the resolved asset is wrapped, but imported at the top-level by this asset,
// then we hoist parcelRequire calls to the top of this asset so side effects run immediately.
if (
Expand Down Expand Up @@ -849,7 +878,7 @@ ${code}
return obj;
}
} else if (
(!staticExports || isWrapped || !symbol) &&
(!staticExports || isWrapped || !symbol || isExternalCommonJS) &&
resolvedAsset !== parentAsset
) {
// If the resolved asset is wrapped or has non-static exports,
Expand Down Expand Up @@ -887,9 +916,7 @@ ${code}
let hoisted = this.hoistedRequires.get(dep.id);
let res = '';
let lineCount = 0;
let isWrapped =
!this.bundle.hasAsset(resolved) ||
(this.wrappedAssets.has(resolved.id) && resolved !== parentAsset);
let isWrapped = this.isWrapped(resolved, parentAsset);

// If the resolved asset is wrapped and is imported in the top-level by this asset,
// we need to run side effects when this asset runs. If the resolved asset is not
Expand Down
Loading