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

dynamic import of modules containing same import statements sometimes result in duplicate code generation #13768

Closed
nikhilnayyar002 opened this issue Jul 11, 2021 · 5 comments

Comments

@nikhilnayyar002
Copy link

nikhilnayyar002 commented Jul 11, 2021

Bug report

What is the current behavior?

If we have two modules say A and B. A and B imports react.
During dynamic import of these two module I found react code to be duplicated.

If the current behavior is a bug, please provide the steps to reproduce.

This is the repo link.

This is the source and public.
image

//src/index.js
import "@styles/index.css";
import "jquery/dist/jquery.slim"

import(/* webpackExports: ["default"] */"./app").then(({default:App})=>{
    import(/* webpackExports: ["default"] */"./app2").then(({default:render})=>{
        render(App)
    })
})
//src/app.js
import { Modal } from 'bootstrap';
import{ useEffect, useState } from 'react';

export default function App() {
    const [state] = useState(Date.now())

    useEffect(()=>{
        if(document.getElementById("myModal")) {
            let elem = document.getElementById("myModal")
            let modal = new Modal(elem)
            modal.hide()
        }
    },[])

    return (
        <div>
            <div>Hello WORLD</div>
            <div>{state}</div>
        </div>
    )
}
//src/app2.js
import React from 'react';
import ReactDOM from 'react-dom';

export default function render(App) {
    ReactDOM.render(
        <React.StrictMode>
            <App />
        </React.StrictMode>,
        document.getElementById('root')
    );
}
//webpack.prod.js
    mode: 'production',
    output: {
        filename: '[name].[contenthash].js',
        chunkFilename:'[id].[contenthash].js',
    },
    plugins: [ ... ],
    optimization: {
        minimize: true,
        minimizer: [
            new TerserPlugin({
                extractComments: false,
            }),
            new CssMinimizerPlugin()
        ],
        moduleIds: 'deterministic',
        runtimeChunk: 'single',
        splitChunks: {
            chunks: 'all',
            cacheGroups: {
                vendorsInitial: {
                    test: /[\\/]node_modules[\\/]/,
                    chunks: 'initial',
                },
                vendorsAsync: {
                    test: /[\\/]node_modules[\\/]/,
                    chunks: 'async',
                },
npm ci
npm run build

These are the build assets:

310.3f191f82792ae2cafdf4.js
589.7d374a5468098e334db1.js
598.dccce690a646df0467cb.js
601.05cbbc0a1fa75c999014.js
70.af0523773e96dec9c092.js
favicon.ico
index.html
main.099275078be1d62b3b50.css
main.aa06ebf00a2dc10cbae3.js
runtime.fd41db16facc83bbd2f9.js
  • index.html
<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
    <meta name="viewport" content="width=device-width,initial-scale=1" />
    <title>React Min</title>
    <link rel="icon" href="/favicon.ico" />
    <link href="/main.099275078be1d62b3b50.css" rel="stylesheet" />
  </head>
  <body>
    <div id="root"></div>
    <script defer="defer" src="/runtime.fd41db16facc83bbd2f9.js"></script>
    <script defer="defer" src="/70.af0523773e96dec9c092.js"></script>
    <script defer="defer" src="/main.aa06ebf00a2dc10cbae3.js"></script>
  </body>
</html>
  • 310.3f191f82792ae2cafdf4.js: this one is src/app.js chunk
  • 589.7d374a5468098e334db1.js: node_modules code chunk for src/app2.js (showing comments only..):
/*
object-assign
(c) Sindre Sorhus
@license MIT
*/
...
/** @license React v17.0.2
 * react-dom.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...
/** @license React v17.0.2
 * react-jsx-runtime.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...
/** @license React v17.0.2
 * react.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...
/** @license React v0.20.2
 * scheduler.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...
  • 598.dccce690a646df0467cb.js: this one is src/app2.js chunk
  • 601.05cbbc0a1fa75c999014.js: node_modules code chunk for src/app.js (showing comments only..):
...  // <-- popperjs code
/*!
  * Bootstrap v5.0.2 (https://getbootstrap.com/)
  * Copyright 2011-2021 The Bootstrap Authors (https://github.com/twbs/bootstrap/graphs/contributors)
  * Licensed under MIT (https://github.com/twbs/bootstrap/blob/main/LICENSE)
  */
...
/*
object-assign
(c) Sindre Sorhus
@license MIT
*/
...
/** @license React v17.0.2
 * react-jsx-runtime.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...
/** @license React v17.0.2
 * react.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...
  • 70.af0523773e96dec9c092.js: this one include jquery code only. Remember we imported jquery in src/index.js. It is a entry module.
  • main.aa06ebf00a2dc10cbae3.js: src/index.js code is included in it. It is a entry module.
  • runtime.fd41db16facc83bbd2f9.js: runtime. It is a entry module.
  • main.099275078be1d62b3b50.css: style imported inside src/index.js .
  • favicon.ico

As one can see these two chunks: 601.05cbbc0a1fa75c999014.js & 589.7d374a5468098e334db1.js has code in common:

/*
object-assign
(c) Sindre Sorhus
@license MIT
*/
...
/** @license React v17.0.2
 * react-jsx-runtime.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...
/** @license React v17.0.2
 * react.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

What is the expected behavior?

Webpack should be able to check that if it has generated chunk for library say React, Bootstrap, React-dom etc then it should not generate code again if it founds that library has been imported inside a dynamically imported module.

Other relevant information:
webpack version: ^5.44.0
Node.js version: 14.17.3
Operating System: Windows 10
Additional tools:

@My69goodtimes
Copy link

9128254029
Duplicate of #

@nikhilnayyar002
Copy link
Author

Which one?

@sokra
Copy link
Member

sokra commented Jul 12, 2021

Yes this might happen as long the shared potion is smaller than 20kb. In this case the optimizer allows duplication to save a request (to the shared chunk). It usually doesn't know that your two import()s are always triggered together. It assumes that each import() may or may not be used based on some condition. It assumes that all import() on the same level are independent.

import(/* webpackExports: ["default"] */"./app").then(({default:App})=>{
    import(/* webpackExports: ["default"] */"./app2").then(({default:render})=>{

This piece of code isn't super optimal anyway, since it will need two round trips to load both apps.

At least you should change it to Promise.all([import("./app"), import("./app2")]), but this would still have the same problem and will still trigger at least two requests.

To solve it you can create a new file apps.js, reexport app and app2: export { default as app } from "./app"; export { default as app2 } from "./app2"; and import that instead: import("./apps").then(({ app: App, app2: render }) => ...).

Note: webpack might be able to understand Promise.all([import("./app"), import("./app2")]) in future, but there are no concrete plans for that...

@nikhilnayyar002
Copy link
Author

nikhilnayyar002 commented Jul 13, 2021

Thanks @sokra.

First Thing

Yeah I understand your point. After doing the changes you suggested, all node_modules library imports of app and app2 are put down in one chunk:

/*!
  * Bootstrap v5.0.2 (https://getbootstrap.com/)
  * Copyright 2011-2021 The Bootstrap Authors (https://github.com/twbs/bootstrap/graphs/contributors)
  * Licensed under MIT (https://github.com/twbs/bootstrap/blob/main/LICENSE)
  */
...
/*
object-assign
(c) Sindre Sorhus
@license MIT
*/
...
/** @license React v17.0.2
 * react-dom.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...
/** @license React v17.0.2
 * react-jsx-runtime.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...
/** @license React v17.0.2
 * react.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...
/** @license React v0.20.2
 * scheduler.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
 ...

But this code:

import(/* webpackExports: ["default"] */"./app").then(({default:App})=>{
    import(/* webpackExports: ["default"] */"./app2").then(({default:render})=>{

I did it intentionally. Why? Well imagine this scenario:

// Import parent dynamically
import("./parent-component").then(({default:ParentComponent})=>{
   // After a button click load child dynamically
    import("./components/child-component").then(({default:ChildComponent})=>{

Here we cannot put both the components in same file. Is'nt?
As you described:

Yes this might happen as long the shared potion is smaller than 20kb. In this case the optimizer allows duplication to save a request (to the shared chunk).

So here I guess we might have to change something in webpack configeration that it do not produce shared code at all.

Imagine we have a 4 chunks: app, app2, app-lib-code, app2-lib-code
app is downloaded and its dependency app-lib-code is fetched as well.
Now app2 is fetched. Its dependency is app2-lib-code is also fetched. Now if app2 find shared code in app-lib-code then app2-lib-code should be small and less bandwidth will be used.

So there is a compromise between bandwidth and request. So your suggestion works well if user has a device with good bandwidth. But if he do not then will it not be better to download less code as possible and reuse a chunk.

Second Thing

Also one more thing as you can see clearly. Bootstrap, React, React-Dom ..., all endup in single file. Ofcourse these modules do also have their own dependencies. Like bootstrap have dependency on popperjs. And I can see their dependency code in the chunk generated. I saw popperjs code, scheduler code (which is dependency of react-dom I guess), object-assign code (dependency of react), jsx-runtime code (dependency of react).

So something like:

import { Modal } from 'bootstrap';

will produce this:

...  // <-- popperjs code (internal bootstrap dependency)
/*!
  * Bootstrap v5.0.2 (https://getbootstrap.com/)
  * Copyright 2011-2021 The Bootstrap Authors (https://github.com/twbs/bootstrap/graphs/contributors)
  * Licensed under MIT (https://github.com/twbs/bootstrap/blob/main/LICENSE)
  */
...  // <-- bootstap.js code 

and something like:

import React from 'react';

will produce this:

/*
object-assign
(c) Sindre Sorhus
@license MIT
*/
... // <-- object-assign code
/** @license React v17.0.2
 * react.production.min.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
...

Can we have a webpack configeration where we have separate chunks for react, react-dom, bootstrap etc?
And is it a good reasoning?

So for eg. if we have somefile.js:

// somefile.js
import { Modal } from 'bootstrap';
import React from 'react';
import ReactDOM from 'react-dom';

that can produce four different chunks like:

bootstrap.chunk.js
react.chunk.js
react-dom.chunk.js
somefile.chunk.js // only developer code

And of course bootstrap.chunk.js will have its code as well as its internal dependency code (popperjs). Similarly for react and react-dom. Now lets say we have one more file say another.js that has this code:

// another.js
import{ useEffect, useState } from 'react';
...
... // jsx code

Now the other code in this file use jsx also. Now if previous file (somefile.js) did not have jsx code then react.chunk.js should not have included code for jsx-runtime since webpack did tree shaking. But now this file wants jsx-runtime to be available. Now there is two options:

  • Include jsx-runtime inside react.chunk.js if possible (for example both the files are part of entry bundle)
  • generate another chunk react_jsx-runtime.chunk.js with only jsx-runtime code only (this can happen if another.js is loaded dynamically). Also this chunk can be reused wherever jsx-runtime is needed.

That's a lot I added. If I am wrong somewhere comments will be helpful. And of course solutions will be great 😉.

@nikhilnayyar002
Copy link
Author

nikhilnayyar002 commented Jul 21, 2021

Hi I saw that by using reducing minSize property value we can generate extra chunk without inlining/duplicating it whereever necessary:

https://webpack.js.org/plugins/split-chunks-plugin/#splitchunksminsize

                vendorsAsync: {
                    test: /[\\/]node_modules[\\/]/,
                    chunks: 'async',
                    minSize: 1024,
                },

Here I used 1KB size (too small 😜) to prevent duplication. Only 1KB size code wil be inlined I guess now. IKB is. Here are the new chunks (you can see now react dependency has a separate chunk now named - 893.0150aab2b3e6d3935707.js ):

index.html
    <!doctype html>
    <html lang="en">

    <head>
        <meta charset="UTF-8" />
        <meta http-equiv="X-UA-Compatible" content="IE=edge" />
        <meta name="viewport" content="width=device-width,initial-scale=1" />
        <title>React Min</title>
        <link rel="icon" href="/favicon.ico">
        <link href="/main.099275078be1d62b3b50.css" rel="stylesheet">
    </head>

    <body>
        <div id="root"></div>
        <script defer="defer" src="/runtime.c117d2a280a587086672.js"></script>
        <script defer="defer" src="/70.af0523773e96dec9c092.js"></script>
        <script defer="defer" src="/main.aa06ebf00a2dc10cbae3.js"></script>
    </body>

    </html>
runtime.60dfb083575ab4b5cbd0.js
    ...
main.099275078be1d62b3b50.css //styles/index.css
    ...
main.7f8b75e61ed51590c250.js //index.js
    ...
70.af0523773e96dec9c092.js //jquery
(self.webpackChunkbase_setup=self.webpackChunkbase_setup||[]).push([[70],{70:function(e,t){var n,r,i;
    /*!
     * jQuery JavaScript Library v3.6.0 -ajax,-ajax/jsonp,-ajax/load,-ajax/script,-ajax/var/location,-ajax/var/nonce,-ajax/var/rquery,-ajax/xhr,-manipulation/_evalUrl,-deprecated/ajax-event-alias,-effects,-effects/Tween,-effects/animatedSelector
     * https://jquery.com/
     *
     * Includes Sizzle.js
     * https://sizzlejs.com/
     *
     * Copyright OpenJS Foundation and other contributors
     * Released under the MIT license
     * https://jquery.org/license
     *
     * Date: 2021-03-02T17:08Z
     */
    ...
    /*!
     * Sizzle CSS Selector Engine v2.3.6
     * https://sizzlejs.com/
     *
     * Copyright JS Foundation and other contributors
     * Released under the MIT license
     * https://js.foundation/
     *
     * Date: 2021-02-16
     */
    ...
598.dccce690a646df0467cb.js //app2.js
    ...
310.3f191f82792ae2cafdf4.js //app.js
    ...
169.01bb91a963e0e3a79f60.js //bootstrap.js
    ...
935.c261eb3988ea53e32424.js //react-dom
    (self.webpackChunkbase_setup=self.webpackChunkbase_setup||[]).push([[935],{448:(e,n,t)=>{
    /** @license React v17.0.2
     * react-dom.production.min.js
     *
     * Copyright (c) Facebook, Inc. and its affiliates.
     *
     * This source code is licensed under the MIT license found in the
     * LICENSE file in the root directory of this source tree.
     */
    ...
    /** @license React v0.20.2
     * scheduler.production.min.js
     *
     * Copyright (c) Facebook, Inc. and its affiliates.
     *
     * This source code is licensed under the MIT license found in the
     * LICENSE file in the root directory of this source tree.
     */
    ...
893.0150aab2b3e6d3935707.js // react
    /*
    object-assign
    (c) Sindre Sorhus
    @license MIT
    */
    ...
    /** @license React v17.0.2
     * react-jsx-runtime.production.min.js
     *
     * Copyright (c) Facebook, Inc. and its affiliates.
     *
     * This source code is licensed under the MIT license found in the
     * LICENSE file in the root directory of this source tree.
     */
    ...
    /** @license React v17.0.2
     * react.production.min.js
     *
     * Copyright (c) Facebook, Inc. and its affiliates.
     *
     * This source code is licensed under the MIT license found in the
     * LICENSE file in the root directory of this source tree.
     */
    ...

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

No branches or pull requests

4 participants