-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Parcel2 v. Webpack - parcel bundle size is significantly larger #4565
Comments
One reason that this library is rather hard to tree shake is that the module exports are assigned to and used inside the declaration, and the non-pure function call in the exports (which makes it even theoretically impossible to shake): function Box() {
console.log(Box.displayName);
return "This is a Box!";
}
Box.displayName = "Box";
export default withSafeTypeForAs(Box); (This This means the either deferring mechanism needs to handle this: export { default as Button } from "./Button";
export * from "./Button";
export { default as Box } from "./Box";
export * from "./Box"; However, the And the wildcard export also prevents Parcel from excluding the asset in the packager (because it determined that We need to rework the import/(re)export wildcard handling. |
Thanks for taking a look! I think you're right to focus on re-export handling. Here's two more data points I found that might be helpful:
|
I also wanted to clarify that there are at least two separate things going wrong in this scenario:
|
I tested some of the @mischnic's hypotheses about what is producing issue 2, and came up with simplified repro. You can see it in the I created a fake npm package (
Then, in the main app, we import an use only one of the exports ( src/index.ts import { message1 } from "fake-package";
ready(() => {
const root = document.getElementById("root");
if (root) {
root.textContent = message1;
}
});
function ready(fn: () => void) {
if (document.readyState != "loading") {
fn();
} else {
document.addEventListener("DOMContentLoaded", fn);
}
} When the bundle is built by webpack with tree-shaking, but without minification ( When the same bundle is built by parcel2 ( Here is the (unminified) bundle out put from parcel: (function () {
// ASSET: /Users/Andrew/Projects/parcel-webpack-comparer/src/node_modules/fake-package/messages.js
// ASSET: /Users/Andrew/Projects/parcel-webpack-comparer/src/node_modules/fake-package/message1.js
// ASSET: /Users/Andrew/Projects/parcel-webpack-comparer/src/node_modules/fake-package/wrapper.js
function $baa56c52e94c37cfa6e4f688d9d4f65$export$wrapper(string) {
console.log("I just wrapped a string: ", string);
return string;
}
const $ddbb96709a4990d332eedf0727c5879$var$message1 = "Hello World!";
var $ddbb96709a4990d332eedf0727c5879$export$default = $baa56c52e94c37cfa6e4f688d9d4f65$export$wrapper(
$ddbb96709a4990d332eedf0727c5879$var$message1
);
const $d3a229d82d83fefab2a3266a3adfcbac$var$message2 = "Goodbye World!";
var $d3a229d82d83fefab2a3266a3adfcbac$export$default = $baa56c52e94c37cfa6e4f688d9d4f65$export$wrapper(
$d3a229d82d83fefab2a3266a3adfcbac$var$message2
);
$b04660a404e967a338647ceb28cef6e$var$ready(function () {
var root = document.getElementById("root");
if (root) {
root.textContent = $ddbb96709a4990d332eedf0727c5879$export$default;
}
});
function $b04660a404e967a338647ceb28cef6e$var$ready(fn) {
if (document.readyState != "loading") {
fn();
} else {
document.addEventListener("DOMContentLoaded", fn);
}
}
})();
//# sourceMappingURL=parcel-webpack-comparer.86a5affc.js.map |
I confirmed that it is possible to fix the problem by adding src/node_modules/fake-package/message1.js import { wrapper } from "./wrapper";
const message1 = "Hello World!";
export default /*#__PURE__*/ wrapper(message1); src/node_modules/fake-package/message2.js import { wrapper } from "./wrapper";
const message2 = "Goodbye World!";
export default /*#__PURE__*/ wrapper(message2); Although a conscientious library maintainer might take the time to test and do this, it seems more likely that many won't (e.g. Another thought: although the repro steps seem pretty specific, I suspect that this might be relatively easy to hit. It's not uncommon to use higher-order-components in react to compose functionality and export the result as
I take the point about tree-shaking this sort of export being incorrect in some cases (e.g. when the wrapping function has side-effects). So I guess there's a tradeoff between matching webpack behavior and being technically correct. I wonder what motivated webpack's decision? I can dig around on github, but does anyone know who the contacts are? |
The pure annotation on the exported function call should not be required. The module in question specifies Using the fake-package in the unannotated example above with the entry point:
compare:
to:
|
Yes. I really meant: Parcel failed to optimize this and the call doesn't make it easier for Terser either |
Rollup handles this |
In general, Parcel handles it as well (#4565 (comment)), it's just that we currently bailout with wildcard exports. (Thank you for magically appearing in minification-related issues BTW) |
Sorry for the noise about this known issue. I saw the "theoretically impossible to shake" comment and didn't realize you already had a strategy to deal with it.
:-) |
I mean AST-level shaking, not bundler level (
Indeed, their performance is very impressive |
Before I forget again: this is part of the problem: parcel/packages/core/integration-tests/test/scope-hoisting.js Lines 746 to 747 in 0bdd83e
|
(I just found out that |
I have implemented the fix and the bundle size is 632kb compared to Webpack's 635kb, will open a PR after more testing. (But 600kb is still massive for a single Button) |
This was motivated by parcel's inability to properly tree shake the Material UI library, as well as its half-baked bundle analysis solutions in v2. parcel-bundler/parcel#4565
This was motivated by parcel's inability to properly tree shake the Material UI library, as well as its half-baked bundle analysis solutions in v2. parcel-bundler/parcel#4565
FYI for anyone re-visiting this issue - recent updates to
...which is much more reasonable (although could be better - part of the problem is that the fluentui theme imports definitions for every possible component, even if you're just using one). See the |
Hopefully soon, this will go down to 500kb and also be a lot faster to build 😉 |
🐛 bug report
I've started trying to use Parcel2 in a real-life situation, and I'm noticed that the parcel bundle size (1.1MB) is significantly larger than webpack's (666 KiB) for the same app. (It's also significantly slower - see #4566).
Like all real-life situations, it's complicated, so I tried to create a simplified repro environment in the
fluentui-button-babel
branch of this repo. It's a simple react app with a singleButton
component from the@fluentui/react-northstar
library. This library is alpha-quality, and there's known issues about bundle size that the team is trying to address. However, webpack still does a significantly better job than parcel right now. The main driver seems to be that this optimization that the library authors made is picked up by webpack but not by parcel.🎛 Configuration (.babelrc, package.json, cli command)
Webpack (full config here):
babel-loader
with this config:production
flag set totrue
, which enables minification throughTerserWebpackPlugin
and scope-hoisting/tree-shaking throughModuleConcatinationPlugin
, among other optimizations.Parcel configuration:
.parcelrc
file, which means that the default babel transformer is being used.parcel build src/index.html
src/index.html:
src/index.tsx:
src/components/App.ts:
🤔 Expected Behavior
Parcel and webpack build size should be roughly the same (or maybe Parcel should be better 😄).
😯 Current Behavior
Here's the bundle size comparison:
babel-loader
with scope-hoisting and minification)ts-loader
withmodule: es6
with scope-hoisting and minification)ts-loader
withmodule: commonjs
with scope-hoisting and minification)💁 Possible Solution
By using
source-map-explorer
and a custom node script, I generated a visualization of the stuff that is in the parcel bundle, but not the webpack bundle:You can see that the main driver of the increased size are icons from the
@fluentui/react-icons-northstar
. The fluentui team recently made an optimization to allow webpack to tree-shake these icons, and it seems that it works with webpack, but not with parcel.I'm not sure why this is happening - most likely something at the scope-hoisting phase that doesn't like the way this library was written. Even if there's something the library authors could do to improve this, it seems like a good goal to do our best to make sure that libraries that are optimized for webpack remain optimized with parcel.
🔦 Context
Trying to build a real-world app that uses
@fluentui/react-northstar
and parcel2.💻 Code Sample
See the
fluentui-button-babel
branch of this repo.🌍 Your Environment
The text was updated successfully, but these errors were encountered: