Additional follow-up fix for #1704 to remove remaining warnings #1736

heath-freenome · 2021-10-20T22:46:48Z

Added devServerMajor.js to razzle-dev-utils that refactors the detection of the webpack-dev-server version in a manner similar to webpackMajor.js
Updated webpackHotDevClient.js to use devServerMajor.js to pickup the properly cased createSocketUrl
- Hopefully this approach won't break things like the change that was made previously that ended up getting reverted
Updated createConfigAsync.js to use razzle-dev-utils/devServerMajor for the webpack-dev-server version as well as:
- Restored hot: true to the common devServer config and instead only explicitly adding the HMR to the plugins for v3 to avoid the following warning:

[webpack-dev-server] "hot: true" automatically applies HMR plugin, you don't have to add it manually to your webpack configuration.

Updated start.js script to include devServerMajor.js so that v4 will start/stop the client dev server rather than listen/close used for v3
- Also added port into the copied devServer config to allow start() to work properly

…nings - Added `devServerMajor.js` to `razzle-dev-utils` that refactors the detection of the `webpack-dev-server` version in a manner similar to `webpackMajor.js` - Updated `webpackHotDevClient.js` to use `devServerMajor.js` to pickup the properly cased `createSocketUrl` - Hopefully this approach won't break things like the change that was made previously that ended up getting reverted - Updated `createConfigAsync.js` to use `razzle-dev-utils/devServerMajor` for the `webpack-dev-server` version as well as: - Restored `hot: true` to the common `devServer` config and instead only explicitly adding the `HMR` to the plugins for v3 to avoid the following warning: ``` [webpack-dev-server] "hot: true" automatically applies HMR plugin, you don't have to add it manually to your webpack configuration. ``` - Updated `start.js` script to include `devServerMajor.js` so that v4 will `start/stop` the client dev server rather than `listen/close` used for v3 - Also added `port` into the copied `devServer` config to allow `start()` to work properly

vercel · 2021-10-20T22:46:52Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/jared/razzle/GE7BiptQE6VwDvkNDE7VcFG6CRdv
✅ Preview: https://razzle-git-fork-heath-freenome-additional-update-t-083719-jared.vercel.app

heath-freenome · 2021-10-20T22:48:12Z

@fivethreeo I believe I fixed everything although I wasn't fully able to verify the webpackHotDevClient change

…fore it is used

…tion

heath-freenome · 2021-10-20T23:36:08Z

@fivethreeo ok, I'm done fixing the little things... Looking forward to your review

heath-freenome · 2021-10-21T20:44:07Z

@fivethreeo Actually, I think this PR (specifically the start.js change) will fix the issue I saw with my App not working with code splitting... Here's why I think it is so:

Using listen() with v4 I get the createSocketURL warning:

WARNING in (webpack)-dev-server/client/utils/createSocketURL.js
There are multiple modules with names that only differ in casing.
This can lead to unexpected behavior when compiling on a filesystem with other case-semantic.
Use equal casing. Compare these module identifiers:
* /Users/heath/dev/razzle/node_modules/webpack-dev-server/client/utils/createSocketURL.js
    Used by 2 module(s), i. e.
    /Users/heath/dev/razzle/node_modules/webpack-dev-server/client/index.js?protocol=ws%3A&hostname=err+%3D%3E+%7B%0A++++++++++++++++if+%28err%29+%7B%0A++++++++++++++++++logger.error%28err%29%3B%0A++++++++++++++++%7D%0A++++++++++++++%7D&port=3001&pathname=%2Fws&logging=none
* /Users/heath/dev/razzle/node_modules/webpack-dev-server/client/utils/createSocketUrl.js
    Used by 1 module(s), i. e.
    /Users/heath/dev/razzle/packages/razzle-dev-utils/webpackHotDevClient.js
 @ (webpack)-dev-server/client/utils/createSocketURL.js
 @ (webpack)-dev-server/client?protocol=ws%3A&hostname=err+%3D%3E+%7B%0A++++++++++++++++if+%28err%29+%7B%0A++++++++++++++++++logger.error%28err%29%3B%0A++++++++++++++++%7D%0A++++++++++++++%7D&port=3001&pathname=%2Fws&logging=none
 @ multi (webpack)-dev-server/client?protocol=ws%3A&hostname=err+%3D%3E+%7B%0A++++++++++++++++if+%28err%29+%7B%0A++++++++++++++++++logger.error%28err%29%3B%0A++++++++++++++++%7D%0A++++++++++++++%7D&port=3001&pathname=%2Fws&logging=none (webpack)/hot/dev-server.js /Users/heath/dev/razzle/packages/razzle-dev-utils/webpackHotDevClient.js ./src/client

If you look closely you can see that hostname is set to some crazy value: hostname=err+%3D%3E+%7B%0A++++++++++++++++if+%28err%29+%7B%0A++++++++++++++++++logger.error%28err%29%3B%0A++++++++++++++++%7D%0A++++++++++++++%7D&

Using start() I see:

WARNING in (webpack)-dev-server/client/utils/createSocketURL.js
There are multiple modules with names that only differ in casing.
This can lead to unexpected behavior when compiling on a filesystem with other case-semantic.
Use equal casing. Compare these module identifiers:
* /Users/heath/dev/razzle/node_modules/webpack-dev-server/client/utils/createSocketURL.js
    Used by 3 module(s), i. e.
    /Users/heath/dev/razzle/node_modules/webpack-dev-server/client/index.js?protocol=ws%3A&hostname=localhost&port=3001&pathname=%2Fws&logging=none
* /Users/heath/dev/razzle/node_modules/webpack-dev-server/client/utils/createSocketUrl.js
    Used by 1 module(s), i. e.
    /Users/heath/dev/razzle/packages/razzle-dev-utils/webpackHotDevClient.js
 @ (webpack)-dev-server/client/utils/createSocketURL.js
 @ (webpack)-dev-server/client?protocol=ws%3A&hostname=localhost&port=3001&pathname=%2Fws&logging=none
 @ multi (webpack)-dev-server/client?protocol=ws%3A&hostname=localhost&port=3001&pathname=%2Fws&logging=none (webpack)/hot/dev-server.js /Users/heath/dev/razzle/packages/razzle-dev-utils/webpackHotDevClient.js ./src/client

As you can see the hostname is properly assigned to localhost... So oddly, that warning helped me fix the breaking issue I was experiencing...

And hopefully the changes I made in webpackHotDevClient.js eliminates the above warning. My local builds still seem to pick up a different version of that client...

tehraven · 2021-10-23T02:33:20Z

packages/razzle/config/createConfigAsync.js

            // See https://github.com/facebookincubator/create-react-app/issues/387.
            disableDotRule: true,
          },
+          hot: true,


tehraven · 2021-10-23T02:38:44Z

packages/razzle/scripts/start.js

-              });
+              if (devServerMajorVersion > 3) {
+                // listen was deprecated in v4 and causes issues when used, switch to its replacement
+                clientDevServer.start();


You could use startCallback if you want to have feature parity on the error handler.

const startCallback = (error) => err && logger.error(err); clientDevServer.startCallback(startCallback , startCallback);

You can add the same (not parity) to stop with stopCallback, which I believe will only trigger if a middleware is unable to close.

Looking at the source code for startCallback and stopCallback currently, (err) will always be null... But I can add it in case things change in the future. Or maybe I'll just add .catch() handlers to both start and stop instead... since ultimately we just want to deal with error that are thrown

Good callout.

webpack/webpack-dev-server#3969

Ok... Pushed code to support this, assuming that the new issue I filed on webpack-dev-server gets addressed.

…`start()` and `stop()` with a generic error handler

chrisdostert · 2021-10-24T18:28:28Z

@fivethreeo For some reason it seems like devServerMajor.js didn't get included in the package released as 4.2.3.

Error: Cannot find module 'razzle-dev-utils/devServerMajor'
 Require stack:
  - /src/node_modules/razzle/config/createConfigAsync.js
  - /src/node_modules/razzle/scripts/start.js

chrisdostert · 2021-10-24T18:30:19Z

@fivethreeo For some reason it seems like devServerMajor.js didn't get included in the package released as 4.2.3.
Error: Cannot find module 'razzle-dev-utils/devServerMajor'
 Require stack:
  - /src/node_modules/razzle/config/createConfigAsync.js
  - /src/node_modules/razzle/scripts/start.js

Ah, I think maybe it needs to be added to files

heath-freenome · 2021-10-24T18:48:11Z

@fivethreeo For some reason it seems like devServerMajor.js didn't get included in the package released as 4.2.3.
Error: Cannot find module 'razzle-dev-utils/devServerMajor'
 Require stack:
  - /src/node_modules/razzle/config/createConfigAsync.js
  - /src/node_modules/razzle/scripts/start.js
Ah, I think maybe it needs to be added to files

@chrisdostert @fivethreeo
#1740

vercel bot deployed to Preview October 20, 2021 22:46 View deployment

- Removed extraneous console.error()

de1903e

vercel bot deployed to Preview October 20, 2021 23:05 View deployment

- Fixed the initialization of createSocketUrl variable to happen be…

2cd8075

…fore it is used

vercel bot deployed to Preview October 20, 2021 23:09 View deployment

- Simplify verbose: verbose down to verbose during object declara…

e03de48

…tion

vercel bot deployed to Preview October 20, 2021 23:16 View deployment

- Fixed spelling in a comment

722186a

vercel bot deployed to Preview October 20, 2021 23:18 View deployment

- Fix another comment

a859278

vercel bot deployed to Preview October 20, 2021 23:37 View deployment

github-actions bot added razzle webpack labels Oct 21, 2021

- Fixed another comment

9754732

vercel bot deployed to Preview October 21, 2021 22:59 View deployment

tehraven reviewed Oct 23, 2021

View reviewed changes

vercel bot deployed to Preview October 23, 2021 16:54 View deployment

- Responded to reviewer feedback to use the **callback** versions of …

8c90455

…`start()` and `stop()` with a generic error handler

heath-freenome force-pushed the additional-update-to-fix-warnings branch from eafd525 to 8c90455 Compare October 23, 2021 16:55

vercel bot deployed to Preview October 23, 2021 16:55 View deployment

fivethreeo merged commit 6cdf807 into jaredpalmer:master Oct 24, 2021

Uh oh!

Additional follow-up fix for #1704 to remove remaining warnings #1736

Additional follow-up fix for #1704 to remove remaining warnings #1736

Uh oh!

Conversation

heath-freenome commented Oct 20, 2021

Uh oh!

vercel bot commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

heath-freenome commented Oct 20, 2021

Uh oh!

heath-freenome commented Oct 20, 2021

Uh oh!

heath-freenome commented Oct 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tehraven Oct 23, 2021

Choose a reason for hiding this comment

Uh oh!

tehraven Oct 23, 2021

Choose a reason for hiding this comment

Uh oh!

tehraven Oct 23, 2021

Choose a reason for hiding this comment

Uh oh!

heath-freenome Oct 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tehraven Oct 23, 2021

Choose a reason for hiding this comment

Uh oh!

heath-freenome Oct 23, 2021

Choose a reason for hiding this comment

Uh oh!

chrisdostert commented Oct 24, 2021

Uh oh!

chrisdostert commented Oct 24, 2021

Uh oh!

heath-freenome commented Oct 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vercel bot commented Oct 20, 2021 •

edited

Loading

heath-freenome commented Oct 21, 2021 •

edited

Loading

heath-freenome Oct 23, 2021 •

edited

Loading