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

Compatibility with Webpack 5 #2802

Merged
merged 36 commits into from
Dec 20, 2020
Merged

Compatibility with Webpack 5 #2802

merged 36 commits into from
Dec 20, 2020

Conversation

gauravtiwari
Copy link
Member

@gauravtiwari gauravtiwari commented Dec 6, 2020

[WIP] This PR introduces a couple of major changes

1. Automatic rules

See the approach in: package/rules/index.js

The idea is that a rule is only loaded if you have declared the relevant dependency in package.json file. This aligns webpacker with the intention of its creation i.e. to only use it for JS compilation. No need to keep extra deps around.

There is still some work left to do to ensure errors are more clear and concise.
Please feel free to add comments.

2. Simpler webpack config

Change from class-based config files to regular object-based config
as seen in webpack documentation to make learning and customisation easier
for everyone.

3. And, webpack 5

Obviously, ensuring the latest release is compatible with Webpack 5.0

Hopefully, I will get this ready and merged this week and then we can push out 6.0 release

Supersedes #2797
See #2757

Out of the box compatibility checklist:

  • JS
  • CSS
  • SASS
  • Tailwindcss
  • React
  • Assets
  • SVG

name: true
},
runtimeChunk: 'single'
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking of leaving òptimization.splitChunks on by default? That would be interesting! Perhaps we might consider then swapping out the implementation of javascript_pack_tag with that of javascript_packs_with_chunks_tag to make that the default behavior.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that makes sense for proper long term caching support (this is recommended in webpack docs)

@rossta
Copy link
Member

rossta commented Dec 10, 2020

@gauravtiwari Thanks for sharing the work-in-progress. I'll give it a closer look this week.

@rossta
Copy link
Member

rossta commented Dec 10, 2020

Good timing. Support for Webpack 5 just released in webpack-assets-manifest.

https://github.com/webdeveric/webpack-assets-manifest/releases/tag/v5.0.0

@gauravtiwari
Copy link
Member Author

thanks @rossta I will get this sorted in the weekend.

@pedrofurtado
Copy link
Member

Happy to see this merged asap! 🎉 🏆

@pedrofurtado
Copy link
Member

A suggestion: Change PR name to "Compatibility with Webpack 5" or something similar 🤝 🍻

@gauravtiwari gauravtiwari changed the title Automatic rules Compatibility with Webpack 5 Dec 16, 2020
@pedrofurtado
Copy link
Member

pedrofurtado commented Dec 18, 2020

Any news about this awesome PR? We can help here testing it in our applications 👍 🏆

@gauravtiwari gauravtiwari merged commit 6ba995a into master Dec 20, 2020
@gauravtiwari gauravtiwari deleted the automatic-rules branch December 20, 2020 17:49
@gauravtiwari
Copy link
Member Author

Hi all,

I have released a new pre-version (6.0):

yarn add @rails/webpacker@6.0.0-pre.2

Gemfile:

gem 'webpacker', '~> 6.x'

Run:

bundle && bundle exec rails webpacker:install

and install any dependencies (if you are using any). For example, to enable CSS support

yarn add css-loader style-loader sass-loader mini-css-extract-plugin

To support postcss, add (plus any plugins in postcss.config):

yarn add postcss-loader

Add to your layouts file (split chunks is now turned on by default):

    <%= stylesheet_packs_with_chunks_tag 'application', media: 'all', 'data-turbolinks-track': 'reload' %>
    <%= javascript_packs_with_chunks_tag 'application', 'data-turbolinks-track': 'reload' %>

There are some breaking changes but hopefully, migration won't be painful :)

Please help me clean up docs.

@gauravtiwari
Copy link
Member Author

Hopefully, we can get 6.0 stable ready just before Christmas 🎄 🎁

return u;
}
});
environment.loaders.get('moduleSass').use = environment.loaders
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @gauravtiwari

should it be loaders.get('moduleSass').use ?

const { environment, loaders } = require('@rails/webpacker')

console.log('environment', environment)
console.log('loaders', loaders)

environment undefined
loaders [
  {
    test: [

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pustovalov Hey, I think we need to overhaul the documentation. The class-based API is removed in 6.x

I will work on updating docs.

@pedrofurtado
Copy link
Member

Hi all,

I have released a new pre-version (6.0):

yarn add @rails/webpacker@6.0.0-pre.2

Gemfile:

gem 'webpacker', '~> 6.x'

Run:

bundle && bundle exec rails webpacker:install

and install any dependencies (if you are using any). For example, to enable CSS support

yarn add css-loader style-loader sass-loader mini-css-extract-plugin

To support postcss, add (plus any plugins in postcss.config):

yarn add postcss-loader

Add to your layouts file (split chunks is now turned on by default):

    <%= stylesheet_packs_with_chunks_tag 'application', media: 'all', 'data-turbolinks-track': 'reload' %>
    <%= javascript_packs_with_chunks_tag 'application', 'data-turbolinks-track': 'reload' %>

There are some breaking changes but hopefully, migration won't be painful :)

Please help me clean up docs.

Thanks @gauravtiwari ! Once docs are updated, to expose the migration guide from 5.x to 6.x version, we can help with testing 🎉

@pixeltrix
Copy link

@gauravtiwari the configuration change from this:

process.env.NODE_ENV = process.env.NODE_ENV || 'development'
const { environment } = require('@rails/webpacker')
module.exports = environment.toWebpackConfig()

to this:

process.env.NODE_ENV = process.env.NODE_ENV || 'development'
const webpackConfig = require('./base')
module.exports = webpackConfig

is going to lead to compilation errors like:

[webpack-cli] TypeError: Cannot read property 'toWebpackConfig' of undefined

in upgrading apps.

What's our strategy for handling this because currently all the relevant stable branches on Rails are broken (@yahonda fixed master by locking to the 5.2.1 npm package version). Is it possible to add a deprecation warning on this and still make the old files work?

@gauravtiwari
Copy link
Member Author

Hi Andrew,

Yes, we can do that. Let me add the old API back in but with warning that it will be removed completely in Webpacker 6.1

Is that sound good to you?

@pixeltrix
Copy link

@gauravtiwari yes, that sounds great - thanks 👍

if current_webpacker_instance.config.extract_css?
stylesheet_link_tag(*sources_from_manifest_entries(names, type: :stylesheet), **options)
end
stylesheet_link_tag(*sources_from_manifest_entries(names, type: :stylesheet), **options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gauravtiwari @rossta why was this removed? If one is using HMR with React, we don't want to extract the CSS. We want the CSS to come from the JS files so that it can be dynamically loaded in the browser.

Copy link
Contributor

@justin808 justin808 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gauravtiwari We can either put back in the extract_css option or we can turn smartly disable extraction when config matches processing matches up (process.env.WEBPACK_DEV_SERVER && process.env.WEBPACK_DEV_SERVER !== 'undefined')

Since the rails process also knows if the dev server is running and the value of the dev_server.hmr, that can smartly pick whether or not to create the view helper tag or leave it blank.


const options = modules ? { include: /\.module\.[a-z]+$/i } : { exclude: /\.module\.[a-z]+$/i }

if (config.extract_css) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gauravtiwari This should be put back. We need this functionality for HMR.

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.

6 participants