-
Notifications
You must be signed in to change notification settings - Fork 42
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
Feature: Add webpack code-splitting to package #97
Comments
@nicholasio Would love to get your insight here as for how we best go about solving this issue :) |
I was just about to report this issue when I saw yours. I want to check this at some point. I discovered with Next (and also happened to me later with Rollup) that having these sort of exports: export { IsAdmin } from './is-admin';
export { Optional } from './optional';
export { InnerBlockSlider } from './inner-block-slider';
export { IconPicker, Icon, IconPickerToolbarButton, InlineIconPicker } from './icon-picker';
export { CustomBlockAppender } from './custom-block-appender';
export { ContentSearch } from './content-search';
export { ContentPicker } from './content-picker';
export { ColorSetting } from './color-settings';
export { ClipboardButton } from './clipboard-button';
export { Link } from './link';
export { MediaToolbar } from './media-toolbar';
export { Image } from './image'; Will prevent tree-shaking from happening and that using this fixed in that case: export * from './is-admin';
export * from './optional';
export * from './inner-block-slider';
export * from './icon-picker';
export * from './custom-block-appender';
export * from './content-search';
export * from './content-picker';
export * from './color-settings';
export * from './clipboard-button';
export * from './link';
export * from './media-toolbar';
export * from './image'; I have tried this here to see if it mitigated the issue but didn't so it requires more investigation. |
So I believe the main issue here is how the package is being bundled. Toolkit ships this package as a single commonjs file, and as far as I know it is not possible to tree-shake that. I've had success creating tree-shakeable libraries where each individual file is transpilled/compiles separately and it's up to the consuming bundler to bundle them for production. When shipping a single build for the whole library I don't think it's possible to do tree-shaking. So our dist folder would need to resemble our actual src. There are a few things we could do:
I think doing the first one in toolkit would be ideal but I haven't thought about the implications. In theory we could just use babel to transpile and not bundle but I'm not sure about the things we would miss by not using webpack. |
@nicholasio it should definitely be possible to tree-shake a single library with a single entry point. I've done that with rollup (for over 60 exports) and I suspect it can be done as well with webpack. I suspect it's due to how things are architectured and laid out rather than being an actual limitation |
@Antonio-Laguna A single entry point or a single bundled file? I was reffering to having something like this: with the entry point being dist/index.js. instead of having a single dist/bundle.js file. I know the first one is tree-shakeable but the second one I haven't had success tree shaking it with webpack. |
I meant both. I will do some investigation. Perhaps it's not doable with Webpack 😂 |
Just wanted to comment on this as an external user of this package. Thanks by the way, this package is excellent 😄 |
It would be great to figure out a way to make the individual components in this package tree shakable. Right now the entire code is being imported when you import a single component and that has a major effect because you suddenly load all of the
@wordrpess/
dependencies due to the dependency extraction plgin.The text was updated successfully, but these errors were encountered: