feat(webpack-dev-server): add `WEBPACK_DEV_SERVER` environment variable by donaldpipowitch · Pull Request #1533 · webpack/webpack-dev-server

donaldpipowitch · 2018-10-18T12:50:25Z

This is a bugfix
This is a code refactor
This is a test update
This is a typo fix
This is a metadata update
This is a feature ← It looks like this is missing in the template? ⚠️

For Bugs and Features; did you add new tests?

No. I just added it in the same way webpack-serve did it and they had no test for this either 🤔As this was an official package before I assume it's okay for this one as well?

Motivation / Use-Case

As described here: #1532.

Breaking Changes

No.

Additional Info

I placed this just before require('webpack-cli/bin/convert-argv') which will require my config and this env variable should be visible for the config file.

👋

codecov · 2018-10-18T13:02:59Z

Codecov Report

Merging #1533 into master will decrease coverage by 13.87%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           master    #1533       +/-   ##
===========================================
- Coverage   89.19%   75.32%   -13.88%     
===========================================
  Files           9       10        +1     
  Lines         611      689       +78     
  Branches      186        0      -186     
===========================================
- Hits          545      519       -26     
- Misses         54      170      +116     
+ Partials       12        0       -12

Impacted Files	Coverage Δ
lib/Server.js	`81.81% <100%> (-3.51%)`	⬇️
bin/webpack-dev-server.js	`58.52% <100%> (ø)`
lib/utils/createLogger.js	`87.5% <0%> (-12.5%)`	⬇️
lib/utils/addEntries.js	`100% <0%> (ø)`	⬆️
lib/utils/createCertificate.js	`100% <0%> (ø)`	⬆️
lib/utils/createDomain.js	`100% <0%> (ø)`	⬆️
lib/utils/updateCompiler.js
lib/utils/defaultTo.js
lib/utils/findPort.js
... and 6 more

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 4c52153...70e2600. Read the comment docs.

alexander-akait

Not right place, we should set WEBPACK_DEV_SERVER when you run using node api too, your code set env var only when you run dev server using cli. Here right place https://github.com/webpack/webpack-dev-server/blob/master/lib/Server.js#L60

donaldpipowitch · 2018-10-18T13:43:45Z

Then I'd set it in both places, because new Server is called too late, if I use the CLI API, if I'm not mistaken. The custom config file would already be required at that stage.

alexander-akait · 2018-10-18T13:54:56Z

@donaldpipowitch too late? Can you describe where it is late?

alexander-akait · 2018-10-18T13:55:19Z

Also need add tests to avoid regressions in future

donaldpipowitch · 2018-10-18T14:06:11Z

Can you describe where it is late?

We need this env var, before the webpack.config.js is required. If I'm not mistaken this happens before new Server() in case of the CLI.

alexander-akait · 2018-10-18T14:13:53Z

@donaldpipowitch looks you right, please add test, also fix lint problem

donaldpipowitch · 2018-10-19T05:58:55Z

Added lint fix and a small test. 👍

alexander-akait

Need test for Server too

donaldpipowitch · 2018-10-19T11:42:34Z

I wasn't sure where I should put that test, so I created a new file. Hope that's fine?

alexander-akait · 2018-10-19T11:55:49Z

Hm, CI tests on windows broken 😕 Not related to this PR

donaldpipowitch · 2018-10-24T05:41:40Z

Anything more I can do? :) Thanks so far.

michael-ciniawsky · 2018-10-25T21:16:38Z

This seems to be currently failing on windows 🤔 https://ci.appveyor.com/project/sokra/webpack-dev-server/builds/19636987/job/0xps5athsnkc5m22

donaldpipowitch · 2018-11-05T06:02:11Z

Is this a failure on my side? 🤔 Hard to tell for me. Should I rebaserebase with the current master?

hiroppy · 2019-04-29T21:28:33Z

@donaldpipowitch Could you rebase? If you don't rebase I will take over.

donaldpipowitch · 2019-04-30T06:38:37Z

I tried to update it. If it doesn't work, I'd be happy if you could move on with the MR.

hiroppy · 2019-05-12T14:50:07Z

@donaldpipowitch Sorry but could you rebase again? We dropped travis and appveyor.

donaldpipowitch requested review from alexander-akait and michael-ciniawsky as code owners October 18, 2018 12:50

alexander-akait requested changes Oct 18, 2018

View reviewed changes

alexander-akait added priority: 4 (nice to have) severity: 4 (inconvenient) semver: minor type: feature pr: rework scope: server labels Oct 18, 2018

michael-ciniawsky changed the title ~~fixes #1532 - set WEBPACK_DEV_SERVER env var~~ feat(webpack-dev-server): add support for WEBPACK_DEV_SERVER env var Oct 18, 2018

donaldpipowitch force-pushed the patch-2 branch from a431080 to 68a23b6 Compare October 19, 2018 05:57

alexander-akait requested changes Oct 19, 2018

View reviewed changes

michael-ciniawsky changed the title ~~feat(webpack-dev-server): add support for WEBPACK_DEV_SERVER env var~~ feat(webpack-dev-server): add WEBPACK_DEV_SERVER environment variable Oct 23, 2018

alexander-akait approved these changes Oct 24, 2018

View reviewed changes

feat(webpack-dev-server): add WEBPACK_DEV_SERVER environment variable

70e2600

donaldpipowitch force-pushed the patch-2 branch from 5542a91 to 70e2600 Compare April 30, 2019 06:38

donaldpipowitch requested a review from hiroppy as a code owner April 30, 2019 06:38

alexander-akait mentioned this pull request May 27, 2019

feat(server): add WEBPACK_DEV_SERVER env variable #1929

Merged

6 tasks

hiroppy closed this in #1929 May 27, 2019

Uh oh!

Uh oh!

Conversation

donaldpipowitch commented Oct 18, 2018 • 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

Breaking Changes

Additional Info

Uh oh!

codecov Bot commented Oct 18, 2018 • 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!

donaldpipowitch commented Oct 18, 2018

Uh oh!

alexander-akait commented Oct 18, 2018

Uh oh!

alexander-akait commented Oct 18, 2018

Uh oh!

donaldpipowitch commented Oct 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

donaldpipowitch commented Oct 19, 2018

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

donaldpipowitch commented Oct 19, 2018

Uh oh!

alexander-akait commented Oct 19, 2018

Uh oh!

donaldpipowitch commented Oct 24, 2018

Uh oh!

michael-ciniawsky commented Oct 25, 2018

Uh oh!

donaldpipowitch commented Nov 5, 2018

Uh oh!

hiroppy commented Apr 29, 2019

Uh oh!

donaldpipowitch commented Apr 30, 2019

Uh oh!

hiroppy commented May 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

donaldpipowitch commented Oct 18, 2018 •

edited

Loading

codecov Bot commented Oct 18, 2018 •

edited

Loading

donaldpipowitch commented Oct 18, 2018 •

edited

Loading

alexander-akait commented Oct 18, 2018 •

edited

Loading