fix(client): add default fallback for client by knagaitsev · Pull Request #2015 · webpack/webpack-dev-server

knagaitsev · 2019-06-11T17:09:24Z

For Bugs and Features; did you add new tests?

Not yet, probably should resolve asap

Motivation / Use-Case

Fixes#2006 by providing a default fallback in case __webpack_dev_server_client__ is undefined.

I moved the default client sources to client/default so that the file path structure is the same, allowing socket.js to require('../clients/SockJSClient') in both client and client-src

Breaking Changes

None

Additional Info

We still need to figure out how to insert __webpack_dev_server_client__ variable into live mode.

codecov · 2019-06-11T17:13:26Z

Codecov Report

Merging #2015 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #2015 +/- ##
=======================================
Coverage 92.77% 92.77% =======================================
Files 29 29 Lines 1149 1149 Branches 327 327 =======================================
Hits 1066 1066 Misses 79 79 Partials 4 4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 303f4e9...d15d7d9. Read the comment docs.

hiroppy · 2019-06-11T22:09:37Z

+// this SockJSClient is here as a default fallback, in case inline mode
+// is off or the client is not injected. This will be switched to
+// WebsocketClient when it becomes the default
+const SockJSClient = require('../clients/SockJSClient');


Move to L19

hiroppy · 2019-06-11T22:27:11Z

We should release as a patch version asap. Thanks.

knagaitsev · 2019-06-12T17:55:12Z

@evilebottnawi Looks good?

alexander-akait

Good job, some notes and question

alexander-akait · 2019-06-13T11:21:18Z

+ // eslint-disable-next-line global-require
+ const SockJSClient = require('../clients/SockJSClient');
+ Client = SockJSClient;
+}


Maybe more simple? Like:
const Client = typeof __webpack_dev_server_client__ !== 'undefined' ? __webpack_dev_server_client__ : require('../clients/SockJSClient');

alexander-akait · 2019-06-13T11:22:32Z

 const sockPort = options.sockPort ? `&sockPort=${options.sockPort}` : '';
 const clientEntry = `${require.resolve(
- '../../client/'
+ '../../client/default/'


Breaking change, some packages can use this directly, so changing path can break these apps

alexander-akait · 2019-06-13T11:22:49Z

 "pretest": "npm run lint",
 "prepare": "rimraf ./ssl/*.pem && npm run build:client",
- "build:client:default": "babel client-src/default --out-dir client --ignore \"./client-src/default/*.config.js\"",
+ "build:client:default": "babel client-src/default --out-dir client/default --ignore \"./client-src/default/*.config.js\"",


Breaking change, as i written above

knagaitsev · 2019-06-13T15:25:51Z

@evilebottnawi any ideas how to make require('../clients/SockJSClient') resolve to the right place in both client and client-src?

alexander-akait · 2019-06-13T15:27:34Z

@Loonride hm, we already keep clients in difference directory, can't see problem

knagaitsev · 2019-06-13T15:38:06Z

@Loonride hm, we already keep clients in difference directory, can't see problem

The problem is the directory organization of client and client-src are not the same. Therefore, when socket.js in client-src uses require('../clients/SockJSClient') it will work

When socket.js in client uses require('../clients/SockJSClient'), it does not work, the correct path would be ./clients/SockJSClient.

And we need both to work, because it needs to be functional in the babel transpiled client, but it also needs to function in client-src so that webpack can compile it to a bundle.

alexander-akait · 2019-06-13T15:45:46Z

Good catch, i think we can change location and create index.js with content like:

require('./default/index')

What do you think?

knagaitsev · 2019-06-13T16:01:32Z

Good catch, i think we can change location and create index.js with content like:
require('./default/index')
What do you think?

That is a good idea, let me push an alternative solution that I just made though which may be simpler and we can compare.

knagaitsev · 2019-06-13T16:11:57Z

@evilebottnawi my idea is to use webpack.NormalModuleReplacementPlugin to direct the paths to the right place. The only problem I have with this is that it makes the files in client-src not work unless accompanied by the proper webpack config. But users should really not be using stuff in client-src anyway, right?

knagaitsev · 2019-06-14T05:08:11Z

@evilebottnawi thoughts?

knagaitsev · 2019-06-17T16:36:18Z

@evilebottnawi @hiroppy Could you take a look so we can resolve this issue?

hiroppy · 2019-06-17T17:11:00Z

 to: path.resolve(__dirname, '../../client/live.html'),
 },
 ]),
+ new webpack.NormalModuleReplacementPlugin(/\/clients\//, (resource) => {


memo: We will delete this plugin at the next major version because we can change the directory structure.

hiroppy · 2019-06-17T23:05:27Z

I'll release today. thanks!

alexander-akait · 2019-06-18T09:16:21Z

Don't do this in future without my approve, here bug, other module(s)/packages can have clients name and we break these apps
/cc @hiroppy @Loonride

alexander-akait · 2019-06-18T11:04:18Z

We should do regexp more strictly now all clients directories in requests rewrites in all modules, i.e. if package use clients directory we rewrite their import/require too, it is very very very big problem, I have many issues about this in DM gutter and other channels and I don't access to fix it right now, please guys take care about it asap, it is incredibly high priority

hiroppy · 2019-06-18T14:12:09Z

@evilebottnawi Would you want to revert? We should add the state actually used by userland to the test.

knagaitsev · 2019-06-18T16:55:55Z

@evilebottnawi Sorry about that, I did not think it would impact other modules.

@hiroppy I say revert, then we follow the other method that @evilebottnawi suggested

fix(client): add default fallback for client
affc96a

knagaitsev requested review from alexander-akait and hiroppy as code owners June 11, 2019 17:09

test(server): update entry snapshot
24d207c

hiroppy suggested changes Jun 11, 2019
View reviewed changes

fix(client): move SockJSClient definition
eb69e4c

hiroppy previously approved these changes Jun 11, 2019
View reviewed changes

wizardofhogwarts previously approved these changes Jun 13, 2019
View reviewed changes

alexander-akait requested changes Jun 13, 2019
View reviewed changes

fix(client): update SockJSClient path resolution
8c90abb

knagaitsev dismissed stale reviews from wizardofhogwarts and hiroppy via 8c90abbJune 13, 2019 16:08

fix(client): add correct entry path
d15d7d9

hiroppy approved these changes Jun 17, 2019
View reviewed changes

Merge branch 'master' into client-definition-fix
055d8f0

hiroppy merged commit d26b444 into webpack:masterJun 17, 2019

knagaitsev mentioned this pull request Jun 18, 2019
fix(client): default client fallback with redirection to all default directory files #2049
Closed
6 tasks

knagaitsev mentioned this pull request Jun 25, 2019
fix(client): stricter reg exp to redirect sockjs client path #2069
Merged
6 tasks

knagaitsev added gsoc Google Summer of Code scope: ws(s) labels Aug 13, 2019

snyk-bot mentioned this pull request Aug 30, 2019
[Snyk] Upgrade webpack-dev-server from 3.2.1 to 3.8.0 iliutastoica/casino-demo-vue-search#4
Closed

snyk-bot mentioned this pull request Nov 1, 2019
[Snyk] Upgrade webpack-dev-server from 3.1.14 to 3.9.0 nbv1982/material-ui#4
Open

This was referenced Dec 14, 2019
[Snyk] Upgrade webpack-dev-server from 3.3.1 to 3.9.0 hamidraza/zcui-vue-pwa#102
Open
[Snyk] Upgrade webpack-dev-server from 3.1.14 to 3.9.0 jakeherp/burger-builder#12
Open

This was referenced Feb 22, 2020
[Snyk] Upgrade webpack-dev-server from 3.2.1 to 3.10.2 saurabharch/react-ecommerce#5
Open
[Snyk] Upgrade webpack-dev-server from 3.1.14 to 3.10.2 raindigi/eShopOnContainers#2
Open

snyk-bot mentioned this pull request Mar 5, 2020
[Snyk] Upgrade webpack-dev-server from 3.1.10 to 3.10.3 sarvaniupadrasta/learnReact#2
Open

vpetersson mentioned this pull request Mar 6, 2020
[Snyk] Upgrade webpack-dev-server from 3.1.14 to 3.10.3 WoTTsecurity/api#757
Closed

snyk-bot mentioned this pull request Mar 21, 2020
[Snyk] Upgrade webpack-dev-server from 2.9.4 to 3.10.3 lwojcik/sc2pte-panel-frontend#85
Closed

snyk-bot mentioned this pull request Apr 4, 2020
[Snyk] Upgrade webpack-dev-server from 3.7.2 to 3.10.3 CollageLabs/collagelabs.org#60
Merged

This was referenced May 5, 2020
[Snyk] Upgrade webpack-dev-server from 3.1.8 to 3.10.3 Barnstorm-Online/barnstorm-cli#6
Open
[Snyk] Upgrade webpack-dev-server from 3.1.14 to 3.10.3 hzhz/material-ui#5
Open

Uh oh!

Uh oh!

Conversation

knagaitsev commented Jun 11, 2019

For Bugs and Features; did you add new tests?

Motivation / Use-Case

Breaking Changes

Additional Info

Uh oh!

codecovBot commented Jun 11, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hiroppyJun 11, 2019

Choose a reason for hiding this comment

Uh oh!

hiroppy commented Jun 11, 2019

Uh oh!

knagaitsev commented Jun 12, 2019

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

alexander-akaitJun 13, 2019

Choose a reason for hiding this comment

Uh oh!

alexander-akaitJun 13, 2019

Choose a reason for hiding this comment

Uh oh!

alexander-akaitJun 13, 2019

Choose a reason for hiding this comment

Uh oh!

knagaitsev commented Jun 13, 2019

Uh oh!

alexander-akait commented Jun 13, 2019

Uh oh!

knagaitsev commented Jun 13, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexander-akait commented Jun 13, 2019

Uh oh!

knagaitsev commented Jun 13, 2019

Uh oh!

knagaitsev commented Jun 13, 2019

Uh oh!

knagaitsev commented Jun 14, 2019

Uh oh!

knagaitsev commented Jun 17, 2019

Uh oh!

Uh oh!

hiroppyJun 17, 2019

Choose a reason for hiding this comment

Uh oh!

hiroppy commented Jun 17, 2019

Uh oh!

alexander-akait commented Jun 18, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexander-akait commented Jun 18, 2019

Uh oh!

hiroppy commented Jun 18, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knagaitsev commented Jun 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecovBot commented Jun 11, 2019•
edited
Loading

knagaitsev commented Jun 13, 2019•
edited
Loading

alexander-akait commented Jun 18, 2019•
edited
Loading

hiroppy commented Jun 18, 2019•
edited
Loading