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

Add library bundler #9489

Merged
merged 13 commits into from
Mar 11, 2024
Merged

Add library bundler #9489

merged 13 commits into from
Mar 11, 2024

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Jan 14, 2024

Fixes #7254

This PR implements a separate bundler plugin for libraries that maps each input file to a separate output bundle, i.e. not bundling at all. This is desirable for libraries because bundlers handle separate files better for tree shaking (via the sideEffects field in package.json). It's especially helpful for component libraries that import CSS, because only the CSS for used components is included rather than everything.

This is currently implemented as a separate bundler plugin (@parcel/bundler-library) that you'd need to manually install and configure to use instead of the default bundler. Maybe this could be the default behavior in v3 though.

This is not fully complete (e.g. doesn't handle dynamic import in libraries which isn't too common), but it is able to successfully build React Spectrum.

if (bundle) {
resolved = potential.find(a => a.type === bundle.type);
}
resolved ||= potential[0];
}

Copy link
Member Author

Choose a reason for hiding this comment

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

The changes in this file are to handle the case where a dependency resolves to multiple bundles. In that case, we need to prioritize the one with the same type as the parent bundle. We also prioritize reference edges over bundle group resolution

for (let b of this.bundleGraph.getReferencedBundles(this.bundle, {
recursive: false,
})) {
this.externals.set(relativeBundlePath(this.bundle, b), new Map());
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This logic ensures that we add imports for all referenced bundles. But we now wait to determine which symbols to import until they are used (below in addExternal). This way, we only include symbols user in this bundle instead of across all bundles.

This also switches to only including bundles non-recursively, otherwise we'd end up hoisting imports for things like CSS even if the library had "sideEffects": false. For a library, there's no need to hoist even with side effects. We leave that to the application bundler consuming the library.

@@ -964,7 +1039,7 @@ ${code}
} else if (!symbol) {
invariant(false, 'Asset was skipped or not found.');
} else {
return symbol;
return replacements?.get(symbol) || symbol;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to check replacements in case a symbol got aliased to an external.

@devongovett devongovett changed the title WIP: Add library bundler Add library bundler Mar 5, 2024
@devongovett devongovett marked this pull request as ready for review March 5, 2024 01:29
@@ -324,7 +309,6 @@ export class ScopeHoistingPackager {

if (
asset.meta.shouldWrap ||
this.isAsyncBundle ||
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be handled by the isAssetReferenced check below. The library bundler makes every bundle "async", but we don't want to wrap those.

@devongovett
Copy link
Member Author

used the export {a as "foo-bar"} syntax for invalid identifiers (tc39/ecma262#2154). still not supported by typescript, but hopefully TS won't be parsing the JS in node_modules anyway. microsoft/TypeScript#49297

@devongovett devongovett merged commit 75182a0 into v2 Mar 11, 2024
14 of 16 checks passed
@devongovett devongovett deleted the library-bundler branch March 11, 2024 20:30
@devongovett
Copy link
Member Author

devongovett commented Mar 16, 2024

@mischnic ugh, looks like the string export syntax breaks webpack 4, which is still very widely used unfortunately. might need to revert that...

@mischnic
Copy link
Member

Looks like we don't have a choice...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to disable bundling for building libraries
2 participants