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 does'n lift common code up to ansector's level #7235

Closed
budarin opened this Issue May 8, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@budarin

budarin commented May 8, 2018

Bug report

What is the current behavior?
After compiling the project many bundles have common code

If the current behavior is a bug, please provide the steps to reproduce.
here is my config:

    mode: 'production',
    target: 'web',
    cache: true,
    entry: {
        client: './src/client/index.js',
    },
    profile: true,
    output: {
        path: path.resolve('./dist'),
        filename: '[name].[hash].js',
        chunkFilename: '[name].[chunkhash].chunk.js',
        publicPath: '/',
    },
    optimization: {
        splitChunks: {
            cacheGroups: {
                default: false,
                vendor: {
                    test: /[\\/]node_modules[\\/]/, // you may add "vendor.js" here if you want to
                    name: 'vendor',
                    chunks: 'all',
                    enforce: true,
                },
            },
        },
        runtimeChunk: {
            name: 'runtime',
        },
        minimizer: [
            new MinifyPlugin(),
            new OptimizeJsPlugin({ sourceMap: false }),
        ],
        occurrenceOrder: true,
    },

What is the expected behavior?
in case of switchin new behavior in wp4 off - webpack should lift all common code on to one level upper in whole bundles hierarchy:
for an example:
(all pages have their own bundles)
We have start page Page1 and 3 pages PageA, PageB & PageC to go from the Page1. If we have some common code in those pages (PageA, PageB & PageC) and default new behavior is switched off - we shode move common code into the Page1.

Other relevant information:
webpack version: 4.8.1
Node.js version: 10.0.0
Operating System: Windows Server 2008R2
Additional tools:

@sokra

This comment has been minimized.

Member

sokra commented May 8, 2018

remove optimization.splitChunks.cacheGroups.default: false. This disables the deduplication of common code. Moving common code to the parent doesn't make sense, because defeats the propose of Code Splitting.

@budarin

This comment has been minimized.

budarin commented May 8, 2018

Why it doesn't make sense??
PageA, PageB & PageC have some shared code and if it's lifted up to Page1 - it's the same as standalone async chunk but without additional request to the server.

If pages have some common code with others in the same level of hierarchy - move code to one level upper than Page1

or is Code Splitting an end in itself at the expense of the flexibility of customization for developers ' needs?

@noahgrant

This comment has been minimized.

noahgrant commented May 9, 2018

I would actually second this request—having a vendor file extracted from your code, for example, is nice, but if it's fetched dynamically via webpack, you have to wait for your javascript to download and execute before the vendor file request is made. On the other hand, if you extracted it into a top-level standalone file, you can add it as a <script> tag in your html file and benefit from lookahead parsing so that the request is made before the initial html file is done even being parsed. That's a big difference!

Of course, I imagine you can take your extracted vendor file and place it into an html file (including the chunk hash) with ExtractTextWebpackPlugin, but in my personal case, we don't use webpack for anything more than javascript bundling, so it'd be nice if there was an option for this for people who don't use webpack for everything.

This could also be solved for me if we could pass a function to the output.chunkFilename option so that I could add a hash to all files except the vendor top-level file (i use a different tool in my workflow to hash my top-level files and add them to the html file).

@budarin

This comment has been minimized.

budarin commented Jun 7, 2018

producing a lot of async common chunks may be usefull for novices, but we dont use webpack in a wild

  • we need full control over our bundles to dynamically include them into HTML on server side
  • also we need to minimize all amount of connections on mobiles.
  • and the last thing - we use PWA so we dont need to use the default webpack's behaviour for novices - we need to have full control over producing bundles and have minimal count of bundles - one per route
@budarin

This comment has been minimized.

budarin commented Jun 24, 2018

by the way, the Parsel by default lift up common dependence.

@sokra

This comment has been minimized.

Member

sokra commented Sep 13, 2018

PageA, PageB & PageC have some shared code and if it's lifted up to Page1 - it's the same as standalone async chunk but without additional request to the server.

Lifting the common code would delay the loading of Page1 for no reason. If in a separate file it can be loaded in parallel to the loading of i. e. PageA.

Lifting code to the parent will always cause unnecessary code to be loaded. You should try to avoid this and only load code when needed. You already told webpack to do this via the import() expression. It basically says "load this module and all dependencies when I call this import()". Lifting common code would break this statement. import() also makes sure that no overhead occurs when this import() is never loaded. Lifting would make this untrue.

@budarin

This comment has been minimized.

budarin commented Sep 14, 2018

produsing a lot of bundles make harder work for adding scripts to a page on server side (discovering on relation to the page)
I agree with your strategy - will struggle with bundles on server side :)

@budarin budarin closed this Sep 14, 2018

@noahgrant

This comment has been minimized.

noahgrant commented Sep 24, 2018

Lifting code to the parent will always cause unnecessary code to be loaded.

@sokra I don't think this is a necessarily-true statement. If you have a dynamic import, that means that it's called from your application javascript code; but in order for that to happen, your application javascript code must have been downloaded and executed. This code is, for single-page apps, often blocking, because without it your page won't work at all. If you have several required such top-level scripts, then by making them blocking you can at least guarantee their order of execution.

Additionally, by making them top-level, you benefit from the browser's look-ahead parser and get a head start on the request. if you have, say, a vendor file that is common across all of your bundles (let's say it's React and lodash), your load times will benefit from keeping that script as a top-level, blocking <script> tag as opposed to a dynamic import fetched in parallel with a chunk—since, of course, a chunk can be much smaller than a vendor file, causing the vendor file to be the limiting request to render.

Anyways, it'd still be desirable for me to be have an option to pull out an app-wide vendor file as top-level, and I imagine it'd be beneficial for other SPA developers for the same reasons. like I mentioned above, though, I could also solve it by having a function for options.chunkFilename.

@noahgrant

This comment has been minimized.

noahgrant commented Oct 6, 2018

@sokra can we reopen this issue? I really think this is something that should be addressed. Thank you!

@noahgrant

This comment has been minimized.

noahgrant commented Nov 1, 2018

ping @sokra. Should I just create a new issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment