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(build): add support for webpack stats JSON output #4189

Merged
merged 1 commit into from
Feb 15, 2017

Conversation

winkerVSbecks
Copy link
Contributor

@winkerVSbecks winkerVSbecks commented Jan 24, 2017

related to #3179

Allows you to run ng build --json which generates dist/stats.json. This can then be used with things like the webpack-bundle-analyzer or https://webpack.github.io/analyse/

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@winkerVSbecks
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@filipesilva
Copy link
Contributor

@hansl WDYT? See also #3179 for context.

I'm fine with the implementation myself.

@clydin
Copy link
Member

clydin commented Jan 24, 2017

Writing to stdout would be useful for automation purposes. other output would need to be disabled though.

If not, then being able to specify the output file would also be useful.

@winkerVSbecks
Copy link
Contributor Author

winkerVSbecks commented Jan 24, 2017

@clydin I'm ok with that. I could add something like this:

  • --stats --json save stats.json
  • --stats --log would log to stdout

I'll let @filipesilva @hansl decide if that is something they want to support.

const jsonStats = stats.toJson('verbose');

fs.writeFileSync(
path.resolve(project.root, outputDir, 'stats.json'),
Copy link

Choose a reason for hiding this comment

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

@winkerVSbecks I think outputDir should be outputPath. There is no variable outputDir. That's what the compiler complains about and why the tests fail in ci.

@winkerVSbecks winkerVSbecks force-pushed the feat-webpack-json branch 3 times, most recently from d1df040 to 7a55eb7 Compare February 7, 2017 16:58
@winkerVSbecks
Copy link
Contributor Author

@hansl @blaugold updated to account for changes

@winkerVSbecks winkerVSbecks force-pushed the feat-webpack-json branch 6 times, most recently from dbf5f2a to c97a0db Compare February 13, 2017 15:53
Copy link
Contributor

@filipesilva filipesilva left a comment

Choose a reason for hiding this comment

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

];

export interface BuildTaskOptions extends BuildOptions {
watch?: boolean;
watcher?: string;
supressSizes: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove these changes? They're redundant and should be obtained from BuildOptions.

@winkerVSbecks
Copy link
Contributor Author

@filipesilva done

@filipesilva
Copy link
Contributor

@winkerVSbecks I don't see the changes in, the interface still has all the duplicated members.

];

export interface BuildTaskOptions extends BuildOptions {
watch?: boolean;
baseHref?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be only

export interface BuildTaskOptions extends BuildOptions {
    watch?: boolean;
    statsJson?: boolean;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, I misunderstood that.

@@ -94,3 +94,5 @@ or `ng serve --prod` will also make use of uglifying and tree-shaking functional
`--extract-css` (`-ec`) extract css from global styles onto css files instead of js ones

`--output-hashing` define the output filename cache-busting hashing mode

`--json` generates a `stats.json` file which can be analyzed using tools such as: `webpack-bundle-analyzer` or https://webpack.github.io/analyse
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change this to --stats-json?

@@ -31,10 +31,12 @@ export const baseBuildCommandOptions: any = [
description: 'define the output filename cache-busting hashing mode',
aliases: ['oh']
},
{ name: 'statsJson', type: Boolean, default: false },
Copy link
Contributor

Choose a reason for hiding this comment

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

The weird CI errors are due to this:

kamik@T460p MINGW64 /d/sandbox/master-project (master)
$ ng build
The "statsJson" option's name of the "build" command contains a capital letter.
Error: The "statsJson" option's name of the "build" command contains a capital letter.
    at Class.Command.validateOption (D:\work\angular-cli\packages\@angular\cli\ember-cli\lib\models\command.js:238:11)

If you change it to stats-json it should be fine.

@filipesilva
Copy link
Contributor

Thanks for implementing this and going through all the change requests!

@filipesilva filipesilva merged commit 469ca91 into angular:master Feb 15, 2017
@winkerVSbecks winkerVSbecks deleted the feat-webpack-json branch February 15, 2017 15:40
asnowwolf pushed a commit to asnowwolf/angular-cli that referenced this pull request Apr 12, 2017
Allows you to run ng build --json which generates dist/stats.json. This can then be used with things like the webpack-bundle-analyzer or https://webpack.github.io/analyse/

Fix angular#3179
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants