fix(addEntries): only add client entry to web targets by mlrawlings · Pull Request #1775 · webpack/webpack-dev-server

mlrawlings · 2019-04-09T00:37:40Z

For Bugs and Features; did you add new tests?

Yes

Motivation / Use-Case

When using an universal/isomorphic multicompiler setup and using the default setting of inline:true, the client runtime gets injected into the entries for each compilation. However, as far as I can tell, the client runtime is only designed to work in the browser (target:"web" or target:"webworker").

With target:"node", the following error is thrown:

urlParts.port = self.location.port;
                ^
ReferenceError: self is not defined

From

webpack-dev-server/client-src/default/index.js

Line 46 in 99e8db0

urlParts.port = self.location.port;

Changes

This PR updates addEntries to check the compilation target for each compiler. Since the client runtime is only designed for web targets, it is only prepended to those compilations. The HMR runtimes are still prepended for all compile targets (when options.hot or options.hotOnly are set), because as far as I can tell, these should work regardless of the compilation target.

Breaking Changes

None, if my assumptions are correct.

Additional Info

Related: #1256 #1338 #1441

jsf-clabot · 2019-04-09T00:37:42Z

All committers have signed the CLA.

codecov · 2019-04-09T00:54:47Z

Codecov Report

Merging #1775 into master will increase coverage by 0.2%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #1775     +/-   ##
=========================================
+ Coverage   87.58%   87.78%   +0.2%     
=========================================
  Files           9        9             
  Lines         596      606     +10     
  Branches      179      184      +5     
=========================================
+ Hits          522      532     +10     
  Misses         62       62             
  Partials       12       12

Impacted Files	Coverage Δ
lib/utils/addEntries.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 58d1682...ca0334d. Read the comment docs.

alexander-akait

Thanks for PR, looks very good 👍

alexander-akait · 2019-04-09T09:58:38Z

/cc @hiroppy

alexander-akait

Interesting note

mlrawlings · 2019-04-09T20:09:41Z

@evilebottnawi I see your concern and I also realized the target can also be a function which complicates things.

I've just pushed a commit that adds two options (plus tests): inlineClient and inlineHot. These options control whether, when inline is true, which configurations get the client runtime and hot runtimes inlined.

new DevServer(compiler, {
  /* default: web targets only */
  inlineClient: (config) => (
    config.target === 'web' ||
    config.target === 'webworker' ||
    config.target == null
  ) 
});

new DevServer(compiler, {
  /* add to all compilers */
  inlineClient: true 
});

new DevServer(compiler, {
  /* add to no compilers */
  inlineClient: false 
});

This would allow users to choose to inline based on any config value. The name option in particular could be useful to target specific compilations.

new DevServer(compiler, {
  /* add to all compilers (as a function) */
  inlineClient: (config) => config.name !== 'Server' 
});

Most users shouldn't ever have to set these options, but they're there if someone needs them. For the vast majority of users, webpack-dev-server should continue to work out-of-the-box.

alexander-akait · 2019-04-09T20:19:48Z

Looks good, but inlineClient and inclideHot is very misleading, maybe injectClient and injectHot?

mlrawlings · 2019-04-09T20:28:09Z

Updated. I like inject better as well.

alexander-akait · 2019-04-09T21:22:49Z

/cc @hiroppy

hiroppy

I confirmed, LGTM. Thank you!

hiroppy · 2019-04-16T19:20:26Z

@mlrawlings Could you rebase?

hiroppy · 2019-04-16T19:21:59Z

BTW, I'll add an example for this after this pr is merged.

alexander-akait · 2019-04-17T18:18:59Z

Thanks great job!

EECOLOR · 2019-04-29T07:57:45Z

Awesome, it took a few hours to see this was the problem, but I was very happy to see it has already been solved and merged. Any indication on when this will be released?

Thanks!

For future reference, the error I received:

 ReferenceError: self is not defined

EECOLOR · 2019-05-08T21:24:14Z

Does it make sense to still have the HotModuleReplacementPlugin injected for non web targets?

FallingSnow · 2023-09-01T02:01:42Z

I'm running into this issue while using web audio workers at

webpack-dev-server/client-src/index.js

Lines 149 to 151 in 540c438

    
           self.addEventListener("beforeunload", () => { 
        
             status.isUnloading = true; 
        
           });

ReferenceError: self is not defined

I have tried the following in my webpack config but the issue remains.

output: {
  globalObject: "globalThis"
}

I also tried setting the target to webworker but the error persists.

snitin315 · 2023-09-01T02:03:30Z

@FallingSnow Can you please create an issue with proper steps to reproduce?

mlrawlings requested review from alexander-akait and hiroppy as code owners April 9, 2019 00:37

mlrawlings force-pushed the universal-inline branch from 7e3cff7 to b03d176 Compare April 9, 2019 02:34

alexander-akait approved these changes Apr 9, 2019

View reviewed changes

alexander-akait reviewed Apr 9, 2019

View reviewed changes

Comment thread lib/utils/addEntries.js

alexander-akait requested changes Apr 9, 2019

View reviewed changes

alexander-akait approved these changes Apr 9, 2019

View reviewed changes

mlrawlings force-pushed the universal-inline branch from b9fde21 to 6bb5d5b Compare April 15, 2019 20:16

hiroppy approved these changes Apr 16, 2019

View reviewed changes

mlrawlings force-pushed the universal-inline branch from 939a1d7 to bf5adc2 Compare April 17, 2019 17:00

fix(addEntries): only add client entry to web and webworker targets

bc2388e

mlrawlings force-pushed the universal-inline branch from bf5adc2 to 858b2eb Compare April 17, 2019 17:13

feat: add injectClient and injectHot options

ca0334d

mlrawlings force-pushed the universal-inline branch from 858b2eb to ca0334d Compare April 17, 2019 17:18

alexander-akait merged commit cf4d0d0 into webpack:master Apr 17, 2019

This was referenced Apr 17, 2019

webpack Multiple Targets #1256

Closed

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

Closed

hiroppy mentioned this pull request Apr 19, 2019

chore(examples): add universal-config example #1810

Merged

6 tasks

Kristonitas mentioned this pull request Jul 5, 2019

Live Reloading and HMR do not work when setting target node-webkit #2026

Closed

2 tasks

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

crawler mentioned this pull request Jan 19, 2021

webpack-dev-server live reload issue fix rails/webpacker#2892

Merged

Uh oh!

Uh oh!

Conversation

mlrawlings commented Apr 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

For Bugs and Features; did you add new tests?

Motivation / Use-Case

Changes

Breaking Changes

Additional Info

Uh oh!

jsf-clabot commented Apr 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Apr 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

alexander-akait commented Apr 9, 2019

Uh oh!

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

mlrawlings commented Apr 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexander-akait commented Apr 9, 2019

Uh oh!

mlrawlings commented Apr 9, 2019

Uh oh!

alexander-akait commented Apr 9, 2019

Uh oh!

hiroppy left a comment

Choose a reason for hiding this comment

Uh oh!

hiroppy commented Apr 16, 2019

Uh oh!

hiroppy commented Apr 16, 2019

Uh oh!

alexander-akait commented Apr 17, 2019

Uh oh!

EECOLOR commented Apr 29, 2019

Uh oh!

EECOLOR commented May 8, 2019

Uh oh!

FallingSnow commented Sep 1, 2023

Uh oh!

snitin315 commented Sep 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mlrawlings commented Apr 9, 2019 •

edited

Loading

jsf-clabot commented Apr 9, 2019 •

edited

Loading

codecov Bot commented Apr 9, 2019 •

edited

Loading

mlrawlings commented Apr 9, 2019 •

edited

Loading

snitin315 commented Sep 1, 2023 •

edited

Loading