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

Update webpack-dev-server config to work with version 4 #3122

Merged
merged 3 commits into from
Aug 25, 2021
Merged

Update webpack-dev-server config to work with version 4 #3122

merged 3 commits into from
Aug 25, 2021

Conversation

t27duck
Copy link
Contributor

@t27duck t27duck commented Aug 20, 2021

webpack-dev-server 4 dropped very recently. With it came a very large set of breaking changes regarding the configuration.

This updates the dev server configurtation in development.js and the default webpacker.yml files to work with it.

I can at least confirm that using this against a relatively basic Rails app using version 4 with tailwind + turbo works as "expected."

The upgrade guide for going from webpacker 5 to 6 pretty much does a redo on the installation (including uninstalling and reinstalling the latest webpack-dev-server), so in theory those on the upgrade path would need to review and reimplement any custom config changes anyway for their webpacker.yml file?

@t27duck
Copy link
Contributor Author

t27duck commented Aug 20, 2021

The failing Ruby tests are the same ones failing in master (the defer attribute on the script tags aren't found) and unrelated to this branch.

@PikachuEXE
Copy link

PikachuEXE commented Aug 20, 2021

For local testing I did

Gemfile:

gem(
  "webpacker",
  ">= 6.0.0.rc1",
  git: "https://github.com/t27duck/webpacker.git",
  branch: "webpack-dev-server-update",
)

package.json

"@rails/webpacker": "https://github.com/t27duck/webpacker.git#webpack-dev-server-update",

Test Execution for webpack-dev-server

ruby ./bin/webpack-dev-server

But I can't terminate with Ctrl+C immediately like v3 (it still terminates but takes 10+ seconds)
Not sure if issue for webpacker or webpack-dev-server

Edit: Looks like the issue is on webpack-dev-server side

@t27duck
Copy link
Contributor Author

t27duck commented Aug 20, 2021

@PikachuEXE - Can't seem to reproduce either. Granted, I was on WSL2 for this branch but it does sound like a webpack-dev-server issue in general.

@AlecRust
Copy link
Contributor

AlecRust commented Aug 20, 2021

When Ctrl+C from the dev server pressing Enter key seems to confirm the exit (there's a prompt that's not worded very well).

This branch worked well in my testing.

Looks like the Ruby tests are fixed on master by f0a29a5 so this branch could do with a rebase.

`webpack-dev-server` 4 dropped very recently. With it came a very large set of breaking changes regarding the configuration.

This updates the dev server configurtation in `development.js` and the default `webpacker.yml` files to work with it.

I can at least confirm that using this against a relatively basic Rails app using version 4 with tailwind + turbo works as "expected."
@t27duck
Copy link
Contributor Author

t27duck commented Aug 20, 2021

Rebased. Now everything is at least green.

@ledermann
Copy link

I have tested this branch with one of my applications, and it appears that some things are still missing. My application uses devServer.public in the webpacker.yml file to configure a local domain instead of localhost:3035. This works with webpacker 6.0.0.rc.1 and webpack-dev-server v3, but not with your branch and v4. You find my repo here:
templatus/templatus-vue#71

v4 supports this, but there are changes in the configuration:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md

It seems to me that devServer.client.webSocketURL should be set for this purpose.

@t27duck
Copy link
Contributor Author

t27duck commented Aug 20, 2021

@ledermann - Thanks for pointing that out. I've made some adjustments to the branch so now pretty much anything in client: in the yaml file will be a pass-thru. I left an example for webSocketURL in the default webpacker.yml file in this branch which should do it for you.

@ledermann
Copy link

@t27duck Cool, this works. But there is one more thing: When setting hmr to true, live reloading is still enabled, which does not make any sense, see here:
https://webpack.js.org/configuration/dev-server/#devserverlivereload

I suggest adding this line to development.js:

liveReload: !devServer.hmr

@t27duck
Copy link
Contributor Author

t27duck commented Aug 21, 2021

@ledermann - Ok, devServer.liveReload should now be set to the inverse of devServer.hot.

(If it's not obvious yet, I tend to run webpacker with defaults so any additional knobs and settings that others need please let me know).

Copy link

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

LGTM after testing the latest change (liveReload value)

@ledermann
Copy link

Hey @dhh, it would be great to get this into the v6 release.

@dhh dhh merged commit d429f02 into rails:master Aug 25, 2021
ignored: '**/node_modules/**'
static:
watch:
ignored: '**/node_modules/**'
Copy link
Contributor

Choose a reason for hiding this comment

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

@t27duck @guillaumebriday Do we need to be on the lookout for changes to the webpacker.yml file as updating this won't be obvious to current installations? Surely not existing beta users. I guess we'll need to double check the upgrade docs one final time before the final release.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, I will open a PR for that to update the upgrade.md file. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@justin808 what I've been telling people in the various discords and slacks I'm in is to treat it like the "upgrade" from 5 to the betas where you back up the original file, take the new default from the gem, and reapply any custom changes.

devServerConfig.client = devServer.client
}

devConfig = merge(devConfig, {

Choose a reason for hiding this comment

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

Doesn't this override all local (user-side) changes to the config though? We're trying to change this config and our changes keep getting overridden by defaults.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What specific changes are you trying to make that are being overridden? I was under the impression that merge would more or less take the existing devConfig which is from the yaml file and stick on the stats and devServer values.

From what I can see, this is a similar pattern to webpacker 4 (old line 21 of this diff)

@PikachuEXE
Copy link

Just found out liveReload is enabled accidentally due to

liveReload: !devServer.hmr

What's supposed to be done when I want both HMR and liveReload disabled?

@t27duck
Copy link
Contributor Author

t27duck commented Sep 2, 2021

@PikachuEXE - Let me know if PR #3154 works for you.

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.

8 participants