Webpack, dev. env: Fix configuration export #10631

jibees · 2023-03-29T08:47:57Z

What? Why?

Since #10568 we changed a bit the configuration options for webpack in development environement. The PR was incomplete, since it does not export the right value. Change that.

What should we test?

Locally, remove public/packs folder
Start your server (via foreman start) and check that after requesting a page, folder public/packs is not empty and you don't have any errors in your browser console
Everything should be fine (can navigate, ...)

Release notes

Changelog Category: Technical changes

dacook · 2023-03-29T22:37:19Z

Oh, sorry I missed that. In fact, apparently there is a lot of release notes that I missed.

dacook

Changes look good, although I have to admit I'm not familiar with webpack configuration.

Dev test also good (just some errors from missing active_storage images which I think is unrelated):

dacook · 2023-03-29T22:41:03Z

config/webpack/development.js


-module.exports = environment.toWebpackConfig()
+module.exports = config;


I see that we were exporting the original config instead of the changed config, so this change makes sense 👍

jibees · 2023-03-30T12:18:10Z

Actually, this seems not solve the original issue: I can still observe the bug with the configuration.
Steps to reproduce (?):

rm -rf public/packs
foreman start
refresh browser
modify app/webpacker/controllers/tom_select_controller.js (I guess it's the same for all js files ...) ; to activate hotmodulereload
refresh the browser
Observe the 404 in the browser console:

GET http://localhost:3000/packs/packs/js/application-e08cb490b1e7d7ef4c24.js 404 (Not Found)
:3000/packs/css/darkswarm-028d6116.css:1
GET http://localhost:3000/packs/css/darkswarm-028d6116.css 404 (Not Found)

or in the rails console:

ActionController::RoutingError (No route matches [GET] "/packs/packs/js/application-9fbe5618e90966ec3586.js"):

jibees · 2023-03-30T13:15:26Z

I spend a lot of time, trying to solve what's happening here, without any success ; actually I'm even not sure on how to reproduce the issue! I think it's great if we could start working on Rails Assets Management pipeline to remove webpack(er) and having a up-to-date assets management system.

Note: from V3 to V4 migrating guide of webpack-dev-server they recommend to use webpack >= 5.

but we recommend using webpack >= v5.0.0

abdellani · 2023-04-02T06:10:22Z

Actually, this seems not solve the original issue: I can still observe the bug with the configuration. Steps to reproduce (?):

rm -rf public/packs

foreman start

refresh browser

modify app/webpacker/controllers/tom_select_controller.js (I guess it's the same for all js files ...) ; to activate hotmodulereload

refresh the browser

Observe the 404 in the browser console:
GET http://localhost:3000/packs/packs/js/application-e08cb490b1e7d7ef4c24.js 404 (Not Found)
:3000/packs/css/darkswarm-028d6116.css:1
GET http://localhost:3000/packs/css/darkswarm-028d6116.css 404 (Not Found)
or in the rails console:
ActionController::RoutingError (No route matches [GET] "/packs/packs/js/application-9fbe5618e90966ec3586.js"):

Hi @jibees
I followed the same steps but I didn't get any errors on the browser's console.

jibees · 2023-04-03T06:24:31Z

Yep, you're right, this does not solve anything. Sorry, that's why this was back to In Dev

jibees · 2023-04-03T08:15:20Z

Remove public/packs
Starts ./bin/webpack-dev-server
Compiled with ddf7a90f84980a3795b1 as hash, manifest.json file is created referencing (it's just an example):

"application": {
      "js": [
        "/packs/packs/js/application-ddf7a90f84980a3795b1.js"
      ],

with the hash ddf7a90f84980a3795b1.

Start the rails server
Request a shopfront
Webpack starts compiling (through the rails app) files with a different hash, but that's fine because manifest.json now reference that new hash:
File created:

public/packs/packs/js/application-18f09577984a69a22fe2.js

manifest.json:

    "application": {
      "js": [
        "/packs/packs/js/application-18f09577984a69a22fe2.js"
      ],

Modify app/webpacker/controllers/tom_select_controller.js but without any modification. Hash is now c82464ad1118ed74b959, as read in the manifest.json file:

    "application": {
      "js": [
        "/packs/packs/js/application-c82464ad1118ed74b959.js"
      ],

No files are created with this hash since compilation has not started through webpack.

Then request the shopfront: no webpack compilation is started, and requesting files fails because of that de-synchronization between webpack-dev-server and webpack hash

ActionController::RoutingError (No route matches [GET] "/packs/packs/js/application-c82464ad1118ed74b959.js"):

(c82464ad1118ed74b959 is the hash in the manifest.json file, but no file had been created with that hash)

My (partial) conclusion is: there seems to have a de-synchronization between webpack-dev-server and webpack around that hash (between manifest.json and files that are actually created).

mkllnk · 2023-04-03T22:54:03Z

My (partial) conclusion is: there seems to have a de-synchronization between webpack-dev-server and webpack around that hash (between manifest.json and files that are actually created).

A hash is usually unique to some input. If you don't change the input then the hash stays the same. So it seems like webpack-dev-server has some changing input which webpack doesn't have. Does it somehow include some already compiled files again? Or is it including a timestamp even though it shouldn't? It could be a bug but these are just theories. I haven't looked into this.

Shall we just abandon webpack-dev-server? If we move to another tool, we won't have that feature anyway, right? It just means that Rails has to recompile with webpack on a new request after changing assets and the browser won't reload automatically.

rioug · 2023-04-04T06:27:13Z

This causing issue for the docker environment as well. Just noting my observation so far :

currently js files are generated in public/packs/packs/js/
the webpage is requesting js files from packs/js but with the wrong hash 😕

If I apply @jibees fix then webpack generate a manifest.json but nothing else 😕

Edit : I think the following fix is working at least in a docker environment, config/webpack/development.js should look like this :

const environment = require('./environment')

const config = environment.toWebpackConfig();
config.output.filename = "js/[name]-[hash].js";

module.exports = config;

jibees · 2023-04-04T12:42:41Z

thanks @rioug for spotting that. I was pretty sure that I already removed that test in my commit, but indeed no. I tried with your suggested solution (which I think I already tested): it does not work for me as expected. I always have some Routing errors and de-synchronization between webpack, rails, ...

rioug · 2023-04-06T00:44:15Z

I played around with this a little bit more with a docker setup. I can see webpack-dev-server recompiling file and updating manifest.json when a file is updated. But it doesn't trigger an auto reload. If I then refresh/load the page webpack will recompile (through the rails app) and pick up the change.
We don't get the benefit of the webpack-dev-server then but at least it's not creating errors.

I realise this is not supper helpful ...

jibees · 2023-04-06T08:17:07Z

I can see webpack-dev-server recompiling file and updating manifest.json when a file is updated. But it doesn't trigger an auto reload. If I then refresh/load the page webpack will recompile (through the rails app) and pick up the change.

Same here (not using docker)

rioug · 2023-04-30T23:46:23Z

Looks like the issue is coming from the upgrade of webpack-dev-server to 4.x and should be fixed by : #10765

rioug · 2023-05-02T06:39:58Z

I rebased and removed the minimum version as we are still on webpack-dev-server 3 it's not applicable to us.

jibees · 2023-05-02T09:51:23Z

Thanks @rioug ! I've tested, and seems to work well. I think this is ready to be reviewed and/or tested.

(oops, sorry, I didn't see that you've put this in Code review and request a review from @dacook . Thanks!)

dacook

Thanks, I can see this is now just a one-line config change, which looks good to me.

It looks like it has already been dev tested, but I'll give it another go.

✅ Looks good, I can see public/packs is created, and local server works. I see a couple of image load errors, but they are active_storage images and probably due to my local data.

✅ Deployed to staging, no console errors.

Ready to merge?

mkllnk

I think that we are trying to fix a workaround which was only introduced by the incompatibility of webpacker versions. We should restore the original file:

process.env.NODE_ENV = process.env.NODE_ENV || 'development'

const environment = require('./environment')

module.exports = environment.toWebpackConfig()

Investigating this I found that we are changing the filename generation to use hash instead of contenthash. But contenthash is recommended and in use by default. I guess that the now reverted version bump changed the naming which brought up this incompatibility. We can now use the default config again.

Sources:

jibees · 2023-05-03T08:06:11Z

@mkllnk
You'r right. Will close this PR (too many comments) and open a new one to bring those changes. Thanks for your attention! 🙏

We used to override the output filename but this was a misunderstanding of an error (due to webpacke(r) incompatibles versions) https://medium.com/@web_developer/hash-vs-chunkhash-vs-contenthash-e94d38a32208 webpack/webpack.js.org#2096 Context: openfoodfoundation#10631 (review)

jibees self-assigned this Mar 29, 2023

jibees force-pushed the fix-webpack-configuration-for-dev-env branch from 341b3ba to 456fc3e Compare March 29, 2023 15:09

dacook approved these changes Mar 29, 2023

View reviewed changes

jibees force-pushed the fix-webpack-configuration-for-dev-env branch from 456fc3e to b9bacba Compare April 3, 2023 08:00

jibees force-pushed the fix-webpack-configuration-for-dev-env branch from b9bacba to 17285a8 Compare April 4, 2023 12:35

rioug mentioned this pull request Apr 30, 2023

Update webpack packages #10765

Merged

rioug self-assigned this May 2, 2023

Fix configuration export

6e34f61

rioug force-pushed the fix-webpack-configuration-for-dev-env branch from 17285a8 to 6e34f61 Compare May 2, 2023 06:06

rioug requested a review from dacook May 2, 2023 06:38

dacook added the pr-staged-au staging.openfoodnetwork.org.au label May 3, 2023

dacook approved these changes May 3, 2023

View reviewed changes

dacook removed the pr-staged-au staging.openfoodnetwork.org.au label May 3, 2023

mkllnk requested changes May 3, 2023

View reviewed changes

jibees closed this May 3, 2023

jibees mentioned this pull request May 3, 2023

Use the default webpack configuration for dev env #10793

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Webpack, dev. env: Fix configuration export #10631

Webpack, dev. env: Fix configuration export #10631

jibees commented Mar 29, 2023 •

edited

Loading

dacook commented Mar 29, 2023

dacook left a comment

dacook Mar 29, 2023

jibees commented Mar 30, 2023 •

edited

Loading

jibees commented Mar 30, 2023

abdellani commented Apr 2, 2023

jibees commented Apr 3, 2023

jibees commented Apr 3, 2023

mkllnk commented Apr 3, 2023

rioug commented Apr 4, 2023 •

edited

Loading

jibees commented Apr 4, 2023

rioug commented Apr 6, 2023

jibees commented Apr 6, 2023

rioug commented Apr 30, 2023

rioug commented May 2, 2023

jibees commented May 2, 2023 •

edited

Loading

dacook left a comment

mkllnk left a comment

jibees commented May 3, 2023


		module.exports = environment.toWebpackConfig()
		module.exports = config;

Webpack, dev. env: Fix configuration export #10631

Webpack, dev. env: Fix configuration export #10631

Conversation

jibees commented Mar 29, 2023 • edited Loading

What? Why?

What should we test?

Release notes

dacook commented Mar 29, 2023

dacook left a comment

Choose a reason for hiding this comment

dacook Mar 29, 2023

Choose a reason for hiding this comment

jibees commented Mar 30, 2023 • edited Loading

jibees commented Mar 30, 2023

abdellani commented Apr 2, 2023

jibees commented Apr 3, 2023

jibees commented Apr 3, 2023

mkllnk commented Apr 3, 2023

rioug commented Apr 4, 2023 • edited Loading

jibees commented Apr 4, 2023

rioug commented Apr 6, 2023

jibees commented Apr 6, 2023

rioug commented Apr 30, 2023

rioug commented May 2, 2023

jibees commented May 2, 2023 • edited Loading

dacook left a comment

Choose a reason for hiding this comment

mkllnk left a comment

Choose a reason for hiding this comment

jibees commented May 3, 2023

jibees commented Mar 29, 2023 •

edited

Loading

jibees commented Mar 30, 2023 •

edited

Loading

rioug commented Apr 4, 2023 •

edited

Loading

jibees commented May 2, 2023 •

edited

Loading