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

Remove NamedModulesPlugin from production builds #1450

Closed
wants to merge 1 commit into from
Closed

Remove NamedModulesPlugin from production builds #1450

wants to merge 1 commit into from

Conversation

levibuzolic
Copy link
Contributor

Looks like Gatsby uses webpack.NamedModulesPlugin() in both production and development modes, diespite this comment in the development case:

Names module ids with their filepath. We use this in development
to make it easier to see what modules have hot reloaded, etc. as
the numerical IDs aren't useful. In production we use numerical module
ids to reduce filesize.

Removing this plugin reduces the output filesize commons bundle from 268KB to 213KB for a single component test site.

Looks like Gatsby uses `webpack.NamedModulesPlugin()` in both production and development modes, diespite this comment in the development case: 

> Names module ids with their filepath. We use this in development
> to make it easier to see what modules have hot reloaded, etc. as
> the numerical IDs aren't useful. In production we use numerical module
> ids to reduce filesize.

Removing this plugin reduces the output filesize of my commons bundle from `268KB` to `213KB`
@KyleAMathews
Copy link
Contributor

Deploy preview failed.

Built with commit b87bfe7

https://app.netlify.com/sites/gatsbyjs/deploys/596372faa700c431db28c60d

@gatsbybot
Copy link
Collaborator

Deploy preview ready!

Built with commit b87bfe7

https://deploy-preview-1450--using-drupal.netlify.com

@gatsbybot
Copy link
Collaborator

Deploy preview ready!

Built with commit b87bfe7

https://deploy-preview-1450--gatsbygram.netlify.com

@KyleAMathews
Copy link
Contributor

What's the gzip difference?

@levibuzolic
Copy link
Contributor Author

levibuzolic commented Jul 11, 2017

gzipped: 63.0KB vs 60.5KB -- about 4% diff

@KyleAMathews
Copy link
Contributor

There's a community member that's working on a new webpack plugin for creating better hashed filenames. Let's just wait for that. The problem with the default numbered module ids is that they change on almost every build and invalidate JS bundles which isn't at all ideal. Since Gzipping takes care of almost all the size difference, I think we're fine.

@KyleAMathews
Copy link
Contributor

Thanks for looking into this! Appreciate your PRs here.

@just-boris
Copy link
Contributor

just-boris commented Sep 27, 2017

Hello!

Recently I have got an issue on my website related to NamedModulePlugin. The reason was that file content (and therefore chunkhash) was not changed, but the build happened on different machine. NamedModule plugin includes absolute paths into module name, for example:

./node_modules/babel-loader/lib/index.js?{"presets":["/opt/build/repo/www/node_modules/babel-preset-es2015/lib/index.js","/opt/build/repo/www/node_modules/babel-preset-stage-1/lib/index.js","/opt/build/repo/www/node_modules/babel-preset-react/lib/index.js",["/opt/build/repo/www/node_modules/babel-preset-env/lib/index.js",{"loose":true,"uglify":true,"modules":"commonjs","targets":{"browsers":["> 1%","last 2 versions","IE >= 9"]},"exclude":["transform-regenerator","transform-es2015-typeof-symbol"]}],"/opt/build/repo/www/node_modules/babel-preset-stage-0/lib/index.js","/opt/build/repo/www/node_modules/babel-preset-react/lib/index.js"],"plugins":["/opt/build/repo/www/node_modules/gatsby/dist/utils/babel-plugin-extract-graphql.js","/opt/build/repo/www/node_modules/babel-plugin-add-module-exports/lib/index.js",["/opt/build/repo/www/node_modules/babel-plugin-transform-react-jsx/lib/index.js",{"pragma":"Glamor.createElement"}],"/opt/build/repo/www/node_modules/babel-plugin-add-module-exports/lib/index.js","/opt/build/repo/www/node_modules/babel-plugin-transform-object-assign/lib/index.js","lodash","glamor/babel-hoist"],"cacheDirectory":true}!./src/pages/docs/ssr-apis.js

taken from gatsby website's static file

When I am using CI system to deploy, it runs my build in random folder, so NamedModule gives it random path every time. But some chunks were cached and have different module name in the content. When I am trying to open website, it gives me the following error:

Uncaught TypeError: Cannot read property 'call' of undefined
    at t (commons-a5ee3297a67817c9d86a.js:1)
    at app-ff75c28302e6d1a4ce2b.js:8
    at app-ff75c28302e6d1a4ce2b.js:4
    at app-ff75c28302e6d1a4ce2b.js:4
    at app-ff75c28302e6d1a4ce2b.js:8
    at i (app-ff75c28302e6d1a4ce2b.js:5)
    at app-ff75c28302e6d1a4ce2b.js:5
    at window.webpackJsonp (commons-a5ee3297a67817c9d86a.js:1)
    at component---src-pages-system-foundation-typography-jsx-c47dc502c5cf6b6c84da.js:1

It is happening in the following webpack internals

// Execute the module function
modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);

moduleId doesn't match with another one that was cached in another file, so it fails there.

Removing NamedModulesPlugin solved the issue for me without any extra problems.

@just-boris
Copy link
Contributor

Another 2c: Webpack documentation says:

This plugin will cause the relative path of the module to be displayed when HMR is enabled. Suggested for use in development.

The purpose of plugin assumes it to be used only in development environment.

@KyleAMathews would it convince you to change the decision regarding this PR?

@KyleAMathews
Copy link
Contributor

@just-boris oh hmmm I'm not actually sure we are using it. Or we are sort of? There's the "HashedChunkIdsPlugin" right below it in the prod config that a community member added. So I'm not sure why the NamedModulesPlugin is still there. Could you try removing it in a PR to see if all the example sites still build?

@just-boris
Copy link
Contributor

just-boris commented Sep 28, 2017

There are two examples:

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

Successfully merging this pull request may close these issues.

None yet

4 participants