devServer v4: options has an unknown property 'before' #88

milosivanovic · 2021-10-02T00:24:33Z

Fixes #83

As per https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md#-breaking-changes, the locations of the websocket parameters for host, port and path have moved. It's important to note that there are TWO types of configurations both specified in the webpack config: the server config, and the client config.

The server config tells webpack-dev-server where to listen, and the client config tells the client how to connect to webpack-dev-server. Under normal circumstances these wouldn't differ, but in the event a proxy is required/preferred for a particular development environment, then the client might need to connect to a different host/port/path in order to reach the server.

These differences are respected in this PR:

If no client configuration is specified, use the server config. (The only hardcoded value here is for the path set to /ws as I'm not sure how to get this default value programmatically from the webpack server config -- the default in the webpacker config is /ws.)
If there is a client configuration specified for host, port or path, then use those values instead.

Tested and working with the following versions:

"webpack": "^5.55.1",
"webpack-cli": "^4.8.0",
"webpack-dev-server": "^4.3.0"

I am seeing this 404 in the developer console though (in my case I'm running webpack-dev-server on port 8081):

I'm not sure what that's referring to or why it's trying to use sockjs when there is already an established websocket at /ws.

thedavidprice · 2021-10-09T05:50:58Z

src/index.js

+      sockOptions.sockHost = compiler.options.devServer.client &&
+                             compiler.options.devServer.client.webSocketURL &&
+                             compiler.options.devServer.client.webSocketURL.hostname || compiler.options.devServer.host
+      sockOptions.sockPath = (
+                               compiler.options.devServer.client &&
+                               compiler.options.devServer.client.webSocketURL &&
+                               compiler.options.devServer.client.webSocketURL.pathname
+                             )
+                             ||
+                             (
+                               compiler.options.devServer.webSocketServer &&
+                               compiler.options.devServer.webSocketServer.options &&
+                               compiler.options.devServer.webSocketServer.options.path
+                             )
+                             || '/ws'
+      sockOptions.sockPort = compiler.options.devServer.client &&
+                             compiler.options.devServer.client.webSocketURL &&
+                             compiler.options.devServer.client.webSocketURL.port || compiler.options.devServer.port


Looks like yarn format needs to be run on this file

looks good!

thedavidprice · 2021-10-09T05:55:00Z

src/index.js

+      sockOptions.sockHost = compiler.options.devServer.client &&
+                             compiler.options.devServer.client.webSocketURL &&
+                             compiler.options.devServer.client.webSocketURL.hostname || compiler.options.devServer.host


Could you use compiler.options.devServer.client?.webSocketURL?.hostname here? Would be more readable if so.

Also, I suggest adding a comment per the description about why this is necessary re: "TWO types of configurations both specified in the webpack config: the server config, and the client config, etc."

thedavidprice

Good work chipping away at this @milosivanovic I left a couple comments that aren't critical to address.

I tried this on an SPA test project using:
webpack 5.57.1
webpack-cli 4.8.0
webpack-dev-server 4.3.1

Dev server ran successfully and is displaying error overlay. However, I'm seeing the same console error along with several others:

These errors are only present when the ErrorOverlayPlugin is used. I did some digging but was unable to diagnose.

The "unexpected token" errors is particularly confusing. Failing the app's index.html... 🤔

Wish I could be of more help but I do hope this adds some positive momentum.

milosivanovic · 2021-10-21T20:25:48Z

@thedavidprice Thanks for your review! I hope I addressed those comments in my latest commit.

The 404 error is strange, and yes, it does only show when the plugin is used. I can say that the functionality works despite the 404, which puts the plugin in a better place than it is on the main branch (not working at all.) We could either try to resolve that in this PR, or commit a subsequent PR after the issue is identified. I'll leave that up for the maintainer to decide.

@gregberge When you have time to check this out, let us know what you think.

thedavidprice · 2021-10-22T19:17:28Z

Changes look good!

I think we need to resolve the 404. But that's up to @gregberge

mrnav90 · 2021-11-08T04:50:16Z

@gregberge
Please help check and merge this PR.
We need this update for webpack 5.

Thanks

iMoses-Apiiro · 2021-12-24T02:42:34Z

any updates? we're waiting on this fix otherwise this package is unusable for us

gondar00 · 2022-02-11T07:34:42Z

Guys can we please get this merged? @thedavidprice @milosivanovic

thedavidprice · 2022-02-11T18:40:11Z

Sorry all, I've no permissions for this repo. And we've now removed the package from RedwoodJS due to the incompatibility so I can't take on responsibility here.

Nudging @gregberge

snitin315 · 2022-02-18T10:51:49Z

src/index.js

-        const originalBefore = options.devServer.before
-        options.devServer.before = (app, server) => {
-          if (originalBefore) {
-            originalBefore(app, server, compiler)
+        const originalOnBeforeSetupMiddleware =
+          options.devServer.onBeforeSetupMiddleware
+        options.devServer.onBeforeSetupMiddleware = (devServer) => {


This broke compatibility with webpack-dev-server v3.

milosivanovic added 2 commits October 1, 2021 17:21

Fix issue gregberge#83

d795c82

Use correct values

e070f72

milosivanovic force-pushed the fix-issue-83 branch from 066980f to 83d418e Compare October 2, 2021 03:25

Respect client config if specified

8c95ee4

milosivanovic force-pushed the fix-issue-83 branch from 83d418e to 8c95ee4 Compare October 2, 2021 03:28

Fix callback

0423045

milosivanovic mentioned this pull request Oct 7, 2021

Options has an unknown property 'before' #83

Closed

thedavidprice reviewed Oct 9, 2021

View reviewed changes

Address PR comments

1e069f6

yasushi-saito mentioned this pull request Feb 9, 2022

ErrorOverlayPlugin causes the devserver schema validator to fail with "options has an unknown property 'before'" webpack/webpack-dev-server#4271

Closed

gregberge approved these changes Feb 12, 2022

View reviewed changes

gregberge merged commit 28de3ed into gregberge:main Feb 12, 2022

milosivanovic deleted the fix-issue-83 branch February 12, 2022 18:07

snitin315 reviewed Feb 18, 2022

View reviewed changes

marlonbrs mentioned this pull request Apr 27, 2022

Maintenance April 2022 BigRoomTechnologies/strangeluv#271

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

devServer v4: options has an unknown property 'before' #88

devServer v4: options has an unknown property 'before' #88

milosivanovic commented Oct 2, 2021 •

edited

Loading

thedavidprice Oct 9, 2021

thedavidprice Oct 22, 2021

thedavidprice Oct 9, 2021

thedavidprice left a comment

milosivanovic commented Oct 21, 2021

thedavidprice commented Oct 22, 2021

mrnav90 commented Nov 8, 2021 •

edited

Loading

iMoses-Apiiro commented Dec 24, 2021

gondar00 commented Feb 11, 2022

thedavidprice commented Feb 11, 2022

snitin315 Feb 18, 2022

devServer v4: options has an unknown property 'before' #88

devServer v4: options has an unknown property 'before' #88

Conversation

milosivanovic commented Oct 2, 2021 • edited Loading

thedavidprice Oct 9, 2021

Choose a reason for hiding this comment

thedavidprice Oct 22, 2021

Choose a reason for hiding this comment

thedavidprice Oct 9, 2021

Choose a reason for hiding this comment

thedavidprice left a comment

Choose a reason for hiding this comment

milosivanovic commented Oct 21, 2021

thedavidprice commented Oct 22, 2021

mrnav90 commented Nov 8, 2021 • edited Loading

iMoses-Apiiro commented Dec 24, 2021

gondar00 commented Feb 11, 2022

thedavidprice commented Feb 11, 2022

snitin315 Feb 18, 2022

Choose a reason for hiding this comment

milosivanovic commented Oct 2, 2021 •

edited

Loading

mrnav90 commented Nov 8, 2021 •

edited

Loading