Skip to content
This repository has been archived by the owner on Jun 1, 2022. It is now read-only.

Duplication in bundle output using skeleton-typescript-webpack (duplicated modules) #894

Closed
chrisckc opened this issue Oct 25, 2018 · 8 comments

Comments

@chrisckc
Copy link

chrisckc commented Oct 25, 2018

I'm submitting a bug report

  • Aurelia Skeleton Version
    skeleton-typescript-webpack
  • Framework Version:
    same as current skeleton-typescript-webpack
    Please tell us about your environment:
  • Operating System:
    MacOS
  • Node Version:
    8.11.4
  • NPM Version:
    6.4.1

Current behavior:
Clone this repo and go to skeleton-typescript-webpack
Inside main.ts, add the line import 'bootstrap
(importing bootstrap is required to allow the bootstrap jQuery extensions to be used in code such as .collapse() etc.)

Run npm start and check bundle contents, notice that the vendor and app bundles now both contain jQuery.

Notice an issue this causes, shrink your browser width so that the hamburger menu kicks in, notice that the hamburger button expands the menu but is no longer able to hide the menu. (this worked before)

I noticed this issue because i wanted to make the hamburger menu collapse after selecting a menu item, which is does not do in skeleton-typescript-webpack. I import bootstrap in main.ts then created a nav-menu.ts file. I added the some jQuery code to nav-menu.ts to make the collapsed menu hide when clicking a menu item. I then noticed that the menu was now broken as it did not hide when clicking the hamburger button. Note that there is another way to do this by adding data-toggle="collapse" to the

  • element inside nav-bar.html but this causes undesirable behaviour which become apparent in later customisations of the nav-bar as it also tries to hide the menu when not collapsed.

    The issue is caused by the duplication of jQuery across both the vendor and app bundles, if you check the bundles after adding import 'bootstrap, you will notice huge bloat in the app bundle and jQuery existing in both bundles.

    jQuery is duplicated along with bootstrap and bluebird due to the use of the following line in webpack.config.js under the entry property:
    vendor: ['bluebird', 'jquery', 'bootstrap'],
    commenting out that line fixes the issue as it allows webpack to sort out the dependancies properly.
    Note that this issue also occurs using webpack v4 and the webpack config generated by the Aurelia CLI.

    Note that commenting out this line results in no vendor bundle being generated and everything being in the app bundle, i believe that for webpack v3, the CommonsChunkPlugin is required to fix that.

    Expected/desired behavior:
    Adding import 'bootstrap should not break the nav-menu. It should not cause duplication of jQuery.
    The webpack config in the skeleton should be correct and allow bootstrap query extensions to be used without causing duplication of jquery bootstrap and bluebird across the vendor and app bundles.

    • What is the motivation / use case for changing the behavior?
      To provide better quality app skeletons for Aurelia devs to use as starting point for projects. To reduce the frustration caused when devs try to do simple things from the app skeleton and find it doesn't work as expected, making them less likely to drop Aurelia in favour of some other popular SPA framework...
  • @chrisckc chrisckc changed the title Bloated bundle output in skeleton-typescript-webpack (duplicated modules) Duplicated bundle output in skeleton-typescript-webpack (duplicated modules) Oct 25, 2018
    @chrisckc chrisckc changed the title Duplicated bundle output in skeleton-typescript-webpack (duplicated modules) Duplication in bundle output using skeleton-typescript-webpack (duplicated modules) Oct 25, 2018
    @chrisckc
    Copy link
    Author

    Here are is the webpack output when vendor: ['bluebird', 'jquery', 'bootstrap'] is in the config file:

    Version: webpack 3.3.0
    Time: 5229ms
                                     Asset       Size  Chunks                    Chunk Names
       fetch.b07976018a493fa2595a.chunk.js      37 kB       0  [emitted]         fetch
      674f50d287a8c48dc19ba404d20fe713.eot     166 kB          [emitted]
     fee66e712a8a08eef5805a46892932ad.woff      98 kB          [emitted]
      b06871f281fee6b241d60582ae9369b9.ttf     166 kB          [emitted]
      912ec66d7572ff821749319396470bde.svg     444 kB          [emitted]  [big]
    af7ae505a9eed503f8b8e6982036873e.woff2    77.2 kB          [emitted]
        app.b07976018a493fa2595a.bundle.js    3.27 MB       1  [emitted]  [big]  app
     vendor.b07976018a493fa2595a.bundle.js    2.69 MB       2  [emitted]  [big]  vendor
                                     1.css     233 kB       1  [emitted]         app
                                index.html  767 bytes          [emitted]
                               favicon.ico    2.25 kB          [emitted]
    

    After importing bootstrap in main.ts

    Version: webpack 3.3.0
    Time: 5301ms
                                     Asset       Size  Chunks                    Chunk Names
       fetch.344e5412d1289f8814aa.chunk.js      37 kB       0  [emitted]         fetch
      674f50d287a8c48dc19ba404d20fe713.eot     166 kB          [emitted]
     fee66e712a8a08eef5805a46892932ad.woff      98 kB          [emitted]
      b06871f281fee6b241d60582ae9369b9.ttf     166 kB          [emitted]
      912ec66d7572ff821749319396470bde.svg     444 kB          [emitted]  [big]
    af7ae505a9eed503f8b8e6982036873e.woff2    77.2 kB          [emitted]
        app.344e5412d1289f8814aa.bundle.js    4.59 MB       1  [emitted]  [big]  app
     vendor.344e5412d1289f8814aa.bundle.js    2.69 MB       2  [emitted]  [big]  vendor
                                     1.css     233 kB       1  [emitted]         app
                                index.html  767 bytes          [emitted]
                               favicon.ico    2.25 kB          [emitted]
    

    Notice that the app bundle is now bigger.

    Whether bootstrap is imported in main.ts or not, commenting out vendor: ['bluebird', 'jquery', 'bootstrap'] results in a much smaller bundles.

    @bigopon
    Copy link
    Member

    bigopon commented Oct 25, 2018

    @EisenbergEffect @chrisckc I can help fix this, if the skeletons are still supported officially. Probably will need to upgrade webpack to v4 too

    @chrisckc
    Copy link
    Author

    Looks like this guy's issue was caused by this too:
    https://discourse.aurelia.io/t/bootstrap-import-bootstrap-breaks-dropdown-menu-in-navbar/641

    @bigopon
    Copy link
    Member

    bigopon commented Oct 25, 2018

    @chrisckc Your config looks nice. Would be awesome if you could create a PR to update the skeletons, if @EisenbergEffect can confirm that we want to maintain them.

    @EisenbergEffect
    Copy link
    Contributor

    I'd like to replace our current skeletons by output from the CLI for standard configs so that we have one source of truth. However, if we aren't quite able to do that with the CLI yet, then we should fix the skeletons. @JeroenVinke can you provide me with a quick update on where we're at with that?

    @JeroenVinke
    Copy link
    Collaborator

    Someone was working on the feature but development stopped due to a personal situation. I’ll pick it back up again starting this weekend. I believe it was almost done. We’d have to add support for auto trace though, since that didn’t exist when work on the feature started

    @EisenbergEffect
    Copy link
    Contributor

    Thanks for looking into this @JeroenVinke Let me know what you think it will take to get us there once you have a chance to assess things.

    @chrisckc
    Copy link
    Author

    chrisckc commented Nov 7, 2018

    @JeroenVinke when you refer to "the feature" do you mean the ability for the CLI to replace the skeletons by updating the CLI to generate the same sample source files (nav-bar.html, welcome.ts, users.ts etc.) as contained in the skeletons?

    In the meantime i think it would be worth updating both the skeletons and the CLI, i will submit a PR for this on the CLI as requested by @EisenbergEffect. I could also do the same for the webpack skeleton if you like?

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

    No branches or pull requests

    4 participants