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

More webpack config updates #1546

Closed
wants to merge 16 commits into from
Closed

More webpack config updates #1546

wants to merge 16 commits into from

Conversation

gauravtiwari
Copy link
Member

@gauravtiwari gauravtiwari commented Jun 3, 2018

In line with Webpack 4 changes, this PR adds support for: split chunking and webpack config updates

  • New split chunking API
const { environment } = require('@rails/webpacker')
// Enable with default config
environment.splitChunks()

// Configure via a callback
environment.splitChunks((config) => Object.assign({}, config, { optimization: { splitChunks: false }}))
  • A separate rule to compile node modules (fixes cases where ES6 libraries were included in the app code)

  • More config updates and tidying up

@gauravtiwari gauravtiwari added the WIP This PR is work in progress label Jun 3, 2018
@edslocomb
Copy link
Contributor

I would get rid of the callback, and make split_chunks an option in webpacker.yml. If users want to fine-tune webpack's config.optimization settings, then let them do that directly by setting split_chunks: false in the .yml file, and applying their own config directly in webpack/*.js

Exposing more of this gem/package's internal configuration API to the user seems like the wrong direction to me.

@renchap
Copy link
Contributor

renchap commented Jun 10, 2018

Babel config should be moved to babel.config.js: https://new.babeljs.io/docs/en/next/babelconfigjs.html
It is much better when you want to have env-specific configs compared to the old .babelrc file.

The new useBuiltIns: "usage", setting for babel-preset-env is also worth a look: https://new.babeljs.io/docs/en/next/babel-preset-env.html#usebuiltins

You could also move the browserlist config to a dedicated file or package.json (https://github.com/browserslist/browserslist#packagejson), it is now supported by babel-preset-env and allows a central point to configure the target browsers.

@gauravtiwari
Copy link
Member Author

I would get rid of the callback, and make split_chunks an option in webpacker.yml. If users want to fine-tune webpack's config.optimization settings, then let them do that directly by setting split_chunks: false in the .yml file, and applying their own config directly in webpack/*.js

@edslocomb Webpacker.yml is intended for configs that are common to both JS package and Ruby Gem - things like paths or extensions. Anything that's related to Webpack can be overridden from environment.js or a specific environment file.

@gauravtiwari
Copy link
Member Author

Thanks @renchap

Yep, been looking into some of these changes. Makes sense to move this to JS to have more control.

In regards to usage vs entry - Looks like entry is similar to the older useBuiltIns option, which does individual requires for the targeted browsers, whereas usage performs static analysis to determine which polyfill is required (babel/babel#6626).

You could also move the browserlist config to a dedicated file or package.json (https://github.com/browserslist/browserslist#packagejson), it is now supported by babel-preset-env and allows a central point to configure the target browsers.

This is already addressed in e25b73b

@gauravtiwari
Copy link
Member Author

Moved out babel changes to #1564

@edslocomb
Copy link
Contributor

@gauravtiwari I'm not sure I understand the reasoning behind that distinction. Surely any option controlling the build, whether it is a pathname or a feature toggle, is a concern of both Webpack and also of the gem that manages Webpack?

I can imagine a user who knows little to nothing about Webpack's config, and wants to keep it that way, using webpacker.yml to toggle a plugin on or off here and there instead of learning Webpack's config syntax.

And I can imagine a user who knows a fair bit about Webpack's config options and config file structure, who wants to alter that file structure directly.

What I can't imagine is a user who wants to learn a third configuration API in addition to those two when trying to get a particular Webpack feature to work through Webpacker.

@ElMassimo
Copy link
Contributor

ElMassimo commented Jun 21, 2018

@edslocomb Agreed, a third configuration API makes no sense, the webpack config should be available for the user to modify easily at will: #1556.

The value in webpacker is in preconfiguring loaders that most apps will require, as well as handling the different release stages as defined in Rails, with helpful tweaks here and there (like the NamedModulesPlugin that is applied in development).

@gauravtiwari Trying to reimplement what is already provided in webpack-chain requires extra-effort for you as a maintainer, and takes that time away from other features that would more directly benefit Webpacker's users 😃

@gauravtiwari gauravtiwari mentioned this pull request Aug 25, 2018
8 tasks
@gauravtiwari gauravtiwari deleted the webpack-config branch December 1, 2018 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP This PR is work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants