feat: allow open option to accept an object by EslamHiko · Pull Request #2492 · webpack/webpack-dev-server

EslamHiko · 2020-04-01T14:09:03Z

For Bugs and Features; did you add new tests?

yes

Motivation / Use-Case

complete : #1770

Breaking Changes

no

Additional Info

I've tested it on ubuntu Linux with this config :

devServer: {
    open: { app: ['google-chrome','--incognito']},
    openPage: 'http://google.com',
  }

codecov · 2020-04-01T14:21:46Z

Codecov Report

Merging #2492 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2492      +/-   ##
==========================================
+ Coverage   93.55%   93.57%   +0.01%     
==========================================
  Files          34       34              
  Lines        1319     1322       +3     
  Branches      379      380       +1     
==========================================
+ Hits         1234     1237       +3     
  Misses         83       83              
  Partials        2        2

Impacted Files	Coverage Δ
lib/utils/runOpen.js	`100.00% <100.00%> (ø)`

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 c6bdfe4...f1eef86. Read the comment docs.

hiroppy · 2020-04-01T14:35:48Z

+      Object.assign({}, argv, {
+        open: {
+          app: ['Google Chrome', '--incognito'],
+        },
+      }),


Suggested change

Object.assign({}, argv, {

open: {

app: ['Google Chrome', '--incognito'],

},

}),

{

...argv,

open: {

app: ['Google Chrome', '--incognito'],

},

},

alexander-akait · 2020-04-01T14:53:11Z

+      ).then(() => {
+        expect(logMock.warn.mock.calls[0][0]).toMatchInlineSnapshot(
+          `"Unable to open \\"https://example.com/index.html\\" in browser: \\"{\\"app\\":[\\"Google Chrome\\",\\"--incognito\\"]}\\". If you are running in a headless environment, please do not use the --open flag"`
+        );


We need better test 😄

I think the best way to test it is manually because even if I found a way to test chrome the chrome start name is different in operating systems ex :
from https://www.npmjs.com/package/open

The app name is platform dependent. Don't hard code it in reusable modules. For example, Chrome is google chrome on macOS, google-chrome on Linux and chrome on Windows.

This is a demo from my machine.

maybe we can use other flag, not good idea keep errors in tests in that case

@evilebottnawi what about replacing it with:
expect(opn).toBeCalledWith('https://example.com/index.html',{ app: ['Google Chrome', '--incognito'] });
This will assure that the right expected arguments has been passed to open

Good idea 👍

alexander-akait · 2020-04-01T18:15:56Z

+        expect(opn).toBeCalledWith('https://example.com/index.html', {
+          wait: false,
+        });
+


Let's return warn check (logMock.warn)

hiroppy · 2020-04-02T03:07:47Z

+      Object.assign(
+        {},


Don't need these lines.

alexander-akait

/cc @hiroppy

hiroppy · 2020-04-02T11:15:22Z

CI is failing.

alexander-akait · 2020-04-02T11:24:29Z

/cc @EslamHiko can you fix CI?

EslamHiko · 2020-04-02T11:30:33Z

@evilebottnawi Yes, I'll do it in another PR

alexander-akait · 2020-04-02T11:35:38Z

@EslamHiko we need fix CI problems in that PR 😄

EslamHiko · 2020-04-02T11:44:40Z

It's related to test/options.test.js because there's TypeError beside the ValidationError

EslamHiko · 2020-04-02T12:23:20Z

/cc @hiroppy @evilebottnawi CI is green now 😅😅 !

alexander-akait

/cc @hiroppy

snitin315 · 2020-04-02T13:05:48Z

This should be documented opening as an issue for the same.

EslamHiko added 2 commits April 1, 2020 16:00

feat: allow open option to accept object

82734a2

test: update snapshot

f7fc490

EslamHiko requested review from alexander-akait, hiroppy and knagaitsev as code owners April 1, 2020 14:09

hiroppy suggested changes Apr 1, 2020

View reviewed changes

EslamHiko added 2 commits April 1, 2020 16:48

test: spread argv

bc68f10

Merge branch 'master' into open-object

d669b69

alexander-akait reviewed Apr 1, 2020

View reviewed changes

test: remove error checks & use toBeCalledWith instead

eaf15d2

alexander-akait reviewed Apr 1, 2020

View reviewed changes

test: return logMock error checks

a69a9e3

hiroppy suggested changes Apr 2, 2020

View reviewed changes

EslamHiko added 2 commits April 2, 2020 08:25

test: remove unnecessary lines

f5a0129

Merge branch 'master' into open-object

5c8eac3

alexander-akait previously approved these changes Apr 2, 2020

View reviewed changes

hiroppy previously approved these changes Apr 2, 2020

View reviewed changes

test: update success & failure types for open option

f1eef86

EslamHiko dismissed stale reviews from hiroppy and alexander-akait via f1eef86 April 2, 2020 11:58

alexander-akait approved these changes Apr 2, 2020

View reviewed changes

hiroppy approved these changes Apr 2, 2020

View reviewed changes

hiroppy merged commit adeb92e into webpack:master Apr 2, 2020

EslamHiko deleted the open-object branch April 2, 2020 12:58

snitin315 mentioned this pull request Apr 2, 2020

docs(devserver.open): add type object. webpack/webpack.js.org#3670

Closed

EslamHiko mentioned this pull request Apr 2, 2020

docs(devserver): webpack-dev-server open new option type webpack/webpack.js.org#3671

Merged

This was referenced Mar 12, 2021

Bump webpack-dev-server from 3.1.10 to 3.11.2 danbruder/ionic-4-elm#7

Merged

Bump webpack-dev-server from 3.10.3 to 3.11.2 boiyelove/ecommerce#16

Closed

Bump webpack-dev-server from 3.1.4 to 3.11.2 in /frontend Quartech/QSLContacts#81

Closed

Uh oh!

Uh oh!

Conversation

EslamHiko commented Apr 1, 2020

For Bugs and Features; did you add new tests?

Motivation / Use-Case

Breaking Changes

Additional Info

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

hiroppy Apr 1, 2020

Choose a reason for hiding this comment

Uh oh!

alexander-akait Apr 1, 2020

Choose a reason for hiding this comment

Uh oh!

EslamHiko Apr 1, 2020 • 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!

alexander-akait Apr 1, 2020

Choose a reason for hiding this comment

Uh oh!

EslamHiko Apr 1, 2020 • 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!

alexander-akait Apr 1, 2020

Choose a reason for hiding this comment

Uh oh!

alexander-akait Apr 1, 2020 • 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!

hiroppy Apr 2, 2020

Choose a reason for hiding this comment

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

hiroppy commented Apr 2, 2020

Uh oh!

alexander-akait commented Apr 2, 2020

Uh oh!

EslamHiko commented Apr 2, 2020

Uh oh!

alexander-akait commented Apr 2, 2020

Uh oh!

EslamHiko commented Apr 2, 2020

Uh oh!

EslamHiko commented Apr 2, 2020

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

snitin315 commented Apr 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Apr 1, 2020 •

edited

Loading

EslamHiko Apr 1, 2020 •

edited

Loading

EslamHiko Apr 1, 2020 •

edited

Loading

alexander-akait Apr 1, 2020 •

edited

Loading