feat(Server): exclude client scripts for `node` targets #1441

EloB · 2018-07-11T17:46:25Z

git clone git@github.com:EloB/ssr.git
cd ssr/node_modules/webpack-dev-server # <- This is using my branch
npm install # Install all dependencies
npm run prepublish # To build all client scripts
cd ../../
npm run dev

Type

Feature

Issues

SemVer

Minor

jsf-clabot · 2018-07-11T17:46:27Z

All committers have signed the CLA.

EloB · 2018-07-11T17:48:19Z

I saw another PR about this: #1338
One problem with that is that the websocket emit hashes that is node targets. That will crash hot reloading on web.

codecov · 2018-07-11T17:49:57Z

Codecov Report

Merging #1441 into master will decrease coverage by 2.92%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1441      +/-   ##
==========================================
- Coverage   79.27%   76.35%   -2.93%     
==========================================
  Files           6        6              
  Lines         497      499       +2     
  Branches      161      163       +2     
==========================================
- Hits          394      381      -13     
- Misses        103      118      +15

Impacted Files	Coverage Δ
lib/Server.js	`77.97% <100%> (-4.3%)`	⬇️
lib/util/addDevServerEntrypoints.js	`100% <100%> (ø)`	⬆️

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 3c1ff11...8328c02. Read the comment docs.

EloB · 2018-07-11T18:05:43Z

lib/Server.js

@@ -68,6 +68,7 @@ function Server(compiler, options, _log) {
  }

  const addCompilerHooks = (comp) => {
+    if (!['async-node', 'node'].includes(comp.options.target)) return;


This is to prevent websocket to not emit node target specific hashes.

alexander-akait

Please add test

EloB · 2018-08-15T11:38:16Z

@evilebottnawi Did you read the description? 😄

Need help writing the tests.

alexander-akait

~~Please add tests~~

alexander-akait · 2018-08-15T11:47:53Z

@EloB oh sorry, i don't have time right now for this 😞 maybe someone other can help with this

EloB · 2018-08-15T11:48:55Z

@evilebottnawi No worry 😄👍 Lets hope someone has time!

michael-ciniawsky · 2018-08-26T00:42:48Z

@EloB See https://github.com/webpack/webpack-dev-server/pull/1338/files#diff-5e0bdc9fe7d56299ec06583d181360a5 for a test case and as a starting point :)

codecov · 2018-09-18T19:12:57Z

Codecov Report

Merging #1441 into master will decrease coverage by 2.92%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1441      +/-   ##
==========================================
- Coverage   79.27%   76.35%   -2.93%     
==========================================
  Files           6        6              
  Lines         497      499       +2     
  Branches      161      163       +2     
==========================================
- Hits          394      381      -13     
- Misses        103      118      +15

Impacted Files	Coverage Δ
lib/Server.js	`77.97% <100%> (-4.3%)`	⬇️
lib/util/addDevServerEntrypoints.js	`100% <100%> (ø)`	⬆️

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 3c1ff11...8328c02. Read the comment docs.

codecov · 2018-09-18T20:49:15Z

Codecov Report

Merging #1441 into master will decrease coverage by 2.92%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1441      +/-   ##
==========================================
- Coverage   79.27%   76.35%   -2.93%     
==========================================
  Files           6        6              
  Lines         497      499       +2     
  Branches      161      163       +2     
==========================================
- Hits          394      381      -13     
- Misses        103      118      +15

Impacted Files	Coverage Δ
lib/Server.js	`77.97% <100%> (-4.3%)`	⬇️
lib/util/addDevServerEntrypoints.js	`100% <100%> (ø)`	⬆️

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 3c1ff11...8328c02. Read the comment docs.

EloB · 2018-10-26T16:47:21Z

If I have some time over this weekend I will try to fix this. I really want that hot reloading on server side :)

alexander-akait · 2018-12-21T10:27:40Z

@EloB friendly ping

j0j00 · 2018-12-21T10:29:55Z

I'm looking to rebase this PR and write some tests for your PR so it can be merged in.

Do you have a sample use-case/webpack config for the websocket emission issue?

I don't think it's a good either to simply skip the compilation hooks in case some plugins rely on them to function.

alexander-akait · 2018-12-21T10:33:13Z

@jojo-apollo why we need test websocket emission issue here?

j0j00 · 2018-12-21T10:39:51Z

This PR supposedly supercedes #1338, by attempting to solve some websocket hot reload issues which that PR doesn't address.

I don't have this websocket issue myself so I'm asking @EloB for clarification on a way to replicate it.

alexander-akait · 2018-12-21T10:50:26Z

@jojo-apollo maybe we can test this manually

j0j00 · 2018-12-21T11:02:52Z

It's difficult to test if we don't know his exact use case, my node target currently uses custom plugins, so I have a feeling his change will break that.

I just want to confirm if it's an issue with way he wrote his webpack config (and he's configured or used it wrong).

https://webpack.js.org/concepts/targets/

alexander-akait · 2018-12-21T11:08:32Z

@jojo-apollo let's wait 72 hours on answer and feel free to recreate PR

EloB · 2019-01-21T13:22:45Z

I managed to write a isomorphic webpack config without any changes or plugins with hot reload on both backend and frontend. When I have time I might be able to publish it.

Exclude client script adding to node targets

8328c02

EloB force-pushed the ssr-friendly branch from 64f30de to 8328c02 Compare July 11, 2018 17:51

EloB commented Jul 11, 2018

View reviewed changes

alexander-akait requested changes Jul 25, 2018

View reviewed changes

alexander-akait requested changes Aug 15, 2018

View reviewed changes

michael-ciniawsky changed the title ~~Exclude client script adding to node targets~~ fix(Server): exclude node targets from being added as a client script Aug 21, 2018

michael-ciniawsky added this to the 3.1.6 milestone Aug 21, 2018

michael-ciniawsky added type: bug priority: 3 (important) severity: 3 (broken) pr: review semver: patch status: pending labels Aug 21, 2018

michael-ciniawsky added this to Fix in Dashboard Aug 25, 2018

michael-ciniawsky changed the title ~~fix(Server): exclude node targets from being added as a client script~~ feat(Server): exclude client scripts for node targets Aug 26, 2018

michael-ciniawsky added semver: minor type: feature and removed semver: patch severity: 3 (broken) type: bug labels Aug 26, 2018

michael-ciniawsky modified the milestones: 3.1.6, 3.2.0 Aug 26, 2018

sanonz mentioned this pull request Aug 26, 2018

feat(addDevServerEntryPoints): exclude client scripts fornode targets #1338

Closed

2 tasks

michael-ciniawsky added the severity: 4 (inconvenient) label Aug 26, 2018

EloB closed this Jan 21, 2019

EloB deleted the ssr-friendly branch January 21, 2019 13:23

mlrawlings mentioned this pull request Apr 9, 2019

fix(addEntries): only add client entry to web targets #1775

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Server): exclude client scripts for `node` targets #1441

feat(Server): exclude client scripts for `node` targets #1441

EloB commented Jul 11, 2018 •

edited by michael-ciniawsky

jsf-clabot commented Jul 11, 2018 •

edited

EloB commented Jul 11, 2018 •

edited

codecov bot commented Jul 11, 2018 •

edited

EloB Jul 11, 2018

alexander-akait left a comment •

edited

EloB commented Aug 15, 2018

alexander-akait left a comment •

edited

alexander-akait commented Aug 15, 2018

EloB commented Aug 15, 2018

michael-ciniawsky commented Aug 26, 2018

codecov bot commented Sep 18, 2018

codecov bot commented Sep 18, 2018 •

edited

EloB commented Oct 26, 2018

alexander-akait commented Dec 21, 2018

j0j00 commented Dec 21, 2018

alexander-akait commented Dec 21, 2018

j0j00 commented Dec 21, 2018

alexander-akait commented Dec 21, 2018

j0j00 commented Dec 21, 2018

alexander-akait commented Dec 21, 2018

EloB commented Jan 21, 2019 •

edited

feat(Server): exclude client scripts for node targets #1441

feat(Server): exclude client scripts for node targets #1441

Conversation

EloB commented Jul 11, 2018 • edited by michael-ciniawsky

Type

Issues

SemVer

jsf-clabot commented Jul 11, 2018 • edited

EloB commented Jul 11, 2018 • edited

codecov bot commented Jul 11, 2018 • edited

Codecov Report

EloB Jul 11, 2018

Choose a reason for hiding this comment

alexander-akait left a comment • edited

Choose a reason for hiding this comment

EloB commented Aug 15, 2018

alexander-akait left a comment • edited

Choose a reason for hiding this comment

alexander-akait commented Aug 15, 2018

EloB commented Aug 15, 2018

michael-ciniawsky commented Aug 26, 2018

codecov bot commented Sep 18, 2018

Codecov Report

codecov bot commented Sep 18, 2018 • edited

Codecov Report

EloB commented Oct 26, 2018

alexander-akait commented Dec 21, 2018

j0j00 commented Dec 21, 2018

alexander-akait commented Dec 21, 2018

j0j00 commented Dec 21, 2018

alexander-akait commented Dec 21, 2018

j0j00 commented Dec 21, 2018

alexander-akait commented Dec 21, 2018

EloB commented Jan 21, 2019 • edited

feat(Server): exclude client scripts for `node` targets #1441

feat(Server): exclude client scripts for `node` targets #1441

EloB commented Jul 11, 2018 •

edited by michael-ciniawsky

jsf-clabot commented Jul 11, 2018 •

edited

EloB commented Jul 11, 2018 •

edited

codecov bot commented Jul 11, 2018 •

edited

alexander-akait left a comment •

edited

alexander-akait left a comment •

edited

codecov bot commented Sep 18, 2018 •

edited

EloB commented Jan 21, 2019 •

edited