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

Webpack isn't treeshaking library bundles #2251

Closed
redallen opened this issue Aug 2, 2019 · 9 comments
Closed

Webpack isn't treeshaking library bundles #2251

redallen opened this issue Aug 2, 2019 · 9 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@redallen
Copy link
Contributor

redallen commented Aug 2, 2019

With only [babel-loader] pointed at an entry: index.js, Webpack will treeshake harmony imports like import { Button } from 'patternfly-react'; and not include the entirety of the patternfly-react project in it's bundle like it is currently:

image

As a few examples, patternfly-react's Wizard is included, along with @patternfly/react-core's Text component.

Really, we should expect a bundle more like:
image (2)

That doesn't include these components.

This will require removing the two require('patternfly-react').then(m => m.Component) in the project as well, since that forces Webpack to bundle the entire module.

@spadgett
Copy link
Member

spadgett commented Aug 9, 2019

@spadgett spadgett added this to the v4.2 milestone Aug 9, 2019
@spadgett
Copy link
Member

/kind bug

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Aug 10, 2019
@spadgett spadgett added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Aug 10, 2019
@spadgett
Copy link
Member

I believe this is specific to the kubevirt plugin, likely kubevirt-web-ui-components. All of the PF3 components drop out of the main vendor bundle when disabling the kubevirt plugin.

cc @vojtechszocs

Current master:

Screen Shot 2019-08-16 at 2 41 17 PM

With kubevirt plugin disabled:

Screen Shot 2019-08-16 at 2 41 28 PM

@spadgett
Copy link
Member

@dgutride fyi

@spadgett
Copy link
Member

@mareklibra fyi

@redallen
Copy link
Contributor Author

Correct, it is due to kubevirt-web-ui-components. That's why I went upstream to provide esm exports for it, and am now using them in #2232 .

@vojtechszocs
Copy link
Contributor

@spadgett @redallen @mareklibra @jelkosz

Correct, it is due to kubevirt-web-ui-components.

This ^^ dependency was added due to migrating code from kubevirt/web-ui fork to Console monorepo, impacting following packages:

  • frontend/packages/kubevirt-plugin
  • frontend/packages/console-shared

After 4.2 gets released and master branch re-opens for development, the KubeVirt group should dedicate some sprints to migrating kubevirt-web-ui-components code over to Console monorepo, resulting in eventual removal of this dependency as well as the fork itself.

As a result, any kubevirt-xxx chunks (as shown in #2394) should become smaller. The whole dependency sub-tree starting at kubevirt-web-ui-components should be cut off entirely.

@spadgett
Copy link
Member

spadgett commented Sep 9, 2019

Fixed by the following PRs:

#2372
#2383
#2595

/close

@openshift-ci-robot
Copy link
Contributor

@spadgett: Closing this issue.

In response to this:

Fixed by the following PRs:

#2372
#2383
#2595

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

4 participants