Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow users to store stats as json to a file #1835

Merged
merged 4 commits into from Oct 3, 2020
Merged

Conversation

snitin315
Copy link
Member

@snitin315 snitin315 commented Sep 25, 2020

What kind of change does this PR introduce?
feature

Did you add tests for your changes?
Yes
If relevant, did you update the documentation?

Summary
Fixes #1535

Screenshot at 2020-09-25 13-32-37

Does this PR introduce a breaking change?
No

Other information

@snitin315 snitin315 marked this pull request as ready for review September 25, 2020 08:20
@snitin315 snitin315 requested a review from a team as a code owner September 25, 2020 08:20
if (outputOptions.json) {
process.stdout.write(JSON.stringify(stats.toJson(outputOptions), null, 2) + '\n');
if (outputOptions.json === true) {
logger.raw(JSON.stringify(stats.toJson(outputOptions), null, 2));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be process.stdout.write

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -69,6 +70,15 @@ class Compiler {
statsErrors.push({ name: statErr.message, loc: errLoc });
});
}
const JSONStats = JSON.stringify(stats.toJson(outputOptions), null, 2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note JSON.stringify doesn't end with a newline.

@alexander-akait
Copy link
Member

/cc @webpack/cli-team

evenstensberg
evenstensberg previously approved these changes Sep 29, 2020
Copy link
Member

@rishabh3112 rishabh3112 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left few suggestions.

packages/webpack-cli/lib/utils/Compiler.js Outdated Show resolved Hide resolved
packages/webpack-cli/lib/utils/cli-flags.js Outdated Show resolved Hide resolved
test/json/json.test.js Show resolved Hide resolved
@@ -53,7 +53,7 @@ yarn add webpack-cli --dev
--no-hot Disables Hot Module Replacement
-d, --devtool string Controls if and how source maps are generated.
--prefetch string Prefetch this request
-j, --json Prints result as JSON
-j, --json string Prints result as JSON or store it in a file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be string, boolean right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, updated

@alexander-akait
Copy link
Member

I think we can improve docs and replace this out of box https://github.com/FormidableLabs/webpack-stats-plugin

@webpack-bot
Copy link

@snitin315 Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@anshumanv Please review the new changes.

@snitin315 snitin315 merged commit 3907517 into next Oct 3, 2020
@snitin315 snitin315 deleted the feat/json branch October 3, 2020 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stats output without stdout
8 participants