feat: remove CLI progress and move progress option into client #2857

ylemkimon · 2020-11-24T12:58:46Z

For Bugs and Features; did you add new tests?

Yes

Motivation / Use-Case

Remove unused bin/options.js in favor of bin/cli-flags.js.
Removed CLI progress output and profile option
Moved progress option to client.progress and added --client-progress.

Breaking Changes

bin/options.js is removed.
profile (--profile) option is removed, to print profile data, set progress: 'profile' (--progress profile).
progress (--progress) option is moved to client, set client: {progress: true} (--client-progress).

Additional Info

Depends on webpack/webpack-cli#2133.

codecov · 2020-11-24T13:05:04Z

Codecov Report

Merging #2857 (0c89036) into v4 (ccdce35) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##               v4    #2857      +/-   ##
==========================================
- Coverage   92.62%   92.61%   -0.01%     
==========================================
  Files          38       38              
  Lines        1247     1246       -1     
  Branches      326      326              
==========================================
- Hits         1155     1154       -1     
  Misses         87       87              
  Partials        5        5

Impacted Files	Coverage Δ
lib/Server.js	`95.46% <100.00%> (-0.02%)`	⬇️

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 ccdce35...0c89036. Read the comment docs.

alexander-akait · 2020-11-24T13:13:01Z

And other simple breaking change:

union https://github.com/webpack/webpack-dev-server/blob/v4/lib/options.json#L295 and https://github.com/webpack/webpack-dev-server/blob/v4/lib/options.json#L298
progress: boolean | 'profile'

I apologize very much for that, I haven't looked into the code for many times and want to reduce complex as much as possible (especial unnecessary options, lets options === better DX)

and progress should be moved in client options, because it is only for client

ylemkimon · 2020-11-24T13:38:26Z

and progress should be moved in client options, because it is only for client

Aren't progress and profile for the webpack compilation?

alexander-akait · 2020-11-24T13:39:49Z

lib/options.json

@@ -292,11 +292,15 @@
        }
      ]
    },
-    "profile": {
-      "type": "boolean"
-    },
    "progress": {


Let's move progress to client object, because we use progress plugin only for client

It seems it's also used for the CLI output and profile is only used for it:

webpack-dev-server/lib/Server.js

Lines 86 to 91 in 9020c23

// for CLI output

new webpack.ProgressPlugin({

profile: this.options.progress === 'profile',

}).apply(this.compiler);

// for browser console output

Aren't progress and profile for the webpack compilation?

For us no, we just emit them to client using sockets

oh, we should remove this, this plugin is unnecessary, webpack serve --progress should enable this plugin on webpack-cli side (like just run webpack --progress), I think it is already works like this, but we need to test it, adding progress plugin is out of scope webpack-dev-server, also if somebody already use this plugin in this configuration we will adding plugin twice

~~Should we add clientProgress or consoleProgress, or would it be too much?~~ I'll move progress to client and remove profile.

Can we add tests for cli too to ensure --progress works fine via CLI for node (for browser I think we already have tests)

Like webpack serve --progress and look at output

~~I cannot find a way to mock stderr with TTY support. I've tried wrapping process.stderr and passing stderr: 'inherit' to execa, but it doesn't work.~~

Why we need mock here? Why just do not run execa and get stderr?

I thought it was TTY issue why it didn't print the progress. However, it seems @webpack-cli/serve doesn't add WebpackCLIPlugin, i.e., doesn't call cli.run() (https://github.com/webpack/webpack-cli/blob/2cd4f1f5d4e99fe5b825d1105e5babe292d99c63/packages/webpack-cli/lib/webpack-cli.js#L595-L597), which is responsible for adding the progress plugin.

bin/cli-flags.js

hiroppy · 2020-11-25T05:17:09Z

bin/cli-flags.js

@@ -106,7 +115,6 @@ module.exports = {
      describe: 'Enable gzip compression',
      group: RESPONSE_GROUP,
    },
-    // findPort is currently not set up


why did you comment out?

The TODO was resolved in #2845.

@ylemkimon Can we remove this?

@evilebottnawi Do you mean removing port from options?

Oh, my mistake, all is fine

alexander-akait · 2020-11-25T11:57:53Z

@ylemkimon @hiroppy Today I want to release [email protected]

Why not alpha? Because I think we should have done it earlier, but unfortunately I did not have enough time for this and I am very sorry
Why not rc or stable? I'm still afraid that we will need to make a pair breaking changes, but we really look stable

I want to hold beta this week and release rc or stable (after feedback) at the next week

What do you think?

hiroppy · 2020-11-25T12:02:41Z

Why not alpha? Because I think we should have done it earlier, but unfortunately I did not have enough time for this and I am very sorry

yes, me too. Not your fault

we have many breaking changes so beta is better I think. I want to see this version for more than 2 weeks because we don't check several platforms such as Electron so we will get many feedbacks..

shaodahong · 2020-11-25T12:28:12Z

Release beta to collect actual usage status, in my local, it's looking good, but can't represent everyone, if release beta I will migrate as soon as possible

alexander-akait · 2020-11-25T14:20:55Z

⭐ I need some improvements on CLI side (webpack serve), ETA is tomorrow

alexander-akait

/cc @hiroppy

alexander-akait · 2020-11-25T14:29:53Z

@ylemkimon My idea right now - move client related options to the client option, for example overlay (https://github.com/webpack/webpack-dev-server/blob/v4/lib/options.json#L264), it is related only for client, why? So that in the future we can extract hmr/client logic into webpack-hot-module-replacement-middleware (maybe better name, feel free to provide good names) and avoid unnecessary breaking changes.

ylemkimon · 2020-11-25T15:35:30Z

lib/Server.js

-      if (this.options.progress) {
-        this.sockWrite([connection], 'progress', this.options.progress);
+      if (this.options.client.progress) {
+        this.sockWrite([connection], 'progress', this.options.client.progress);
      }

      if (this.options.clientOverlay) {


I just realized this should be this.options.client.overlay.

yes, let's fix it in the separate PR

ylemkimon · 2020-11-25T15:39:17Z

@ylemkimon My idea right now - move client related options to the client option, for example overlay (https://github.com/webpack/webpack-dev-server/blob/v4/lib/options.json#L264), it is related only for client.

What do you think about:

injectClient
injectHot
useLocalIp

alexander-akait · 2020-11-26T11:20:15Z

@ylemkimon I need think about it, let's focus on overlay

ylemkimon · 2020-11-26T14:25:32Z

Should I open a new PR for overlay or update this PR?

alexander-akait · 2020-11-26T14:27:31Z

@ylemkimon New PR

alexander-akait · 2020-11-26T14:27:38Z

Let's merge this

fix: remove unused bin/options.js

ddfb1f6

ylemkimon requested review from alexander-akait, hiroppy and knagaitsev as code owners November 24, 2020 12:58

ylemkimon changed the title ~~fix: remove unused bin/options.js~~ feat(options): merge profile option into progress Nov 24, 2020

alexander-akait reviewed Nov 24, 2020

View reviewed changes

ylemkimon added 4 commits November 24, 2020 23:01

refactor: resolve TODOs

1155e23

feat: remove CLI progress output and profile option

d23c4ee

fix: move progress option to client.progress

0a7b363

fix(cli): add processor for hot-only and client-logging

0c89036

ylemkimon force-pushed the patch-2 branch from 9020c23 to 0c89036 Compare November 24, 2020 18:04

ylemkimon changed the title ~~feat(options): merge profile option into progress~~ feat: remove CLI progress output and move profile option into client Nov 24, 2020

ylemkimon changed the title ~~feat: remove CLI progress output and move profile option into client~~ feat: remove CLI progress and move progress option into client Nov 24, 2020

alexander-akait reviewed Nov 24, 2020

View reviewed changes

bin/cli-flags.js Show resolved Hide resolved

hiroppy reviewed Nov 25, 2020

View reviewed changes

alexander-akait approved these changes Nov 25, 2020

View reviewed changes

hiroppy approved these changes Nov 25, 2020

View reviewed changes

ylemkimon commented Nov 25, 2020

View reviewed changes

alexander-akait merged commit ecf2fe1 into webpack:v4 Nov 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remove CLI progress and move progress option into client #2857

feat: remove CLI progress and move progress option into client #2857

ylemkimon commented Nov 24, 2020 •

edited

Loading

codecov bot commented Nov 24, 2020 •

edited

Loading

alexander-akait commented Nov 24, 2020 •

edited

Loading

ylemkimon commented Nov 24, 2020

alexander-akait Nov 24, 2020 •

edited

Loading

ylemkimon Nov 24, 2020 •

edited

Loading

alexander-akait Nov 24, 2020 •

edited

Loading

alexander-akait Nov 24, 2020 •

edited

Loading

ylemkimon Nov 24, 2020 •

edited

Loading

alexander-akait Nov 24, 2020

alexander-akait Nov 24, 2020

ylemkimon Nov 24, 2020 •

edited

Loading

alexander-akait Nov 24, 2020

ylemkimon Nov 24, 2020 •

edited

Loading

hiroppy Nov 25, 2020

ylemkimon Nov 25, 2020

alexander-akait Nov 25, 2020

ylemkimon Nov 25, 2020

alexander-akait Nov 25, 2020 •

edited

Loading

alexander-akait commented Nov 25, 2020

hiroppy commented Nov 25, 2020 •

edited

Loading

shaodahong commented Nov 25, 2020

alexander-akait commented Nov 25, 2020

alexander-akait left a comment

alexander-akait commented Nov 25, 2020

ylemkimon Nov 25, 2020

alexander-akait Nov 26, 2020

ylemkimon commented Nov 25, 2020

alexander-akait commented Nov 26, 2020

ylemkimon commented Nov 26, 2020

alexander-akait commented Nov 26, 2020

alexander-akait commented Nov 26, 2020

	// for CLI output
	new webpack.ProgressPlugin({
	profile: this.options.progress === 'profile',
	}).apply(this.compiler);

	// for browser console output

feat: remove CLI progress and move progress option into client #2857

feat: remove CLI progress and move progress option into client #2857

Conversation

ylemkimon commented Nov 24, 2020 • edited Loading

For Bugs and Features; did you add new tests?

Motivation / Use-Case

Breaking Changes

Additional Info

codecov bot commented Nov 24, 2020 • edited Loading

Codecov Report

alexander-akait commented Nov 24, 2020 • edited Loading

ylemkimon commented Nov 24, 2020

alexander-akait Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

ylemkimon Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

alexander-akait Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

alexander-akait Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

ylemkimon Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ylemkimon Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ylemkimon Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-akait Nov 25, 2020 • edited Loading

Choose a reason for hiding this comment

alexander-akait commented Nov 25, 2020

hiroppy commented Nov 25, 2020 • edited Loading

shaodahong commented Nov 25, 2020

alexander-akait commented Nov 25, 2020

alexander-akait left a comment

Choose a reason for hiding this comment

alexander-akait commented Nov 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ylemkimon commented Nov 25, 2020

alexander-akait commented Nov 26, 2020

ylemkimon commented Nov 26, 2020

alexander-akait commented Nov 26, 2020

alexander-akait commented Nov 26, 2020

ylemkimon commented Nov 24, 2020 •

edited

Loading

codecov bot commented Nov 24, 2020 •

edited

Loading

alexander-akait commented Nov 24, 2020 •

edited

Loading

alexander-akait Nov 24, 2020 •

edited

Loading

ylemkimon Nov 24, 2020 •

edited

Loading

alexander-akait Nov 24, 2020 •

edited

Loading

alexander-akait Nov 24, 2020 •

edited

Loading

ylemkimon Nov 24, 2020 •

edited

Loading

ylemkimon Nov 24, 2020 •

edited

Loading

ylemkimon Nov 24, 2020 •

edited

Loading

alexander-akait Nov 25, 2020 •

edited

Loading

hiroppy commented Nov 25, 2020 •

edited

Loading