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

Add new script to generate bundle-stats.json. #3928

Closed
wants to merge 27 commits into from

Conversation

joshwcomeau
Copy link
Contributor

@joshwcomeau joshwcomeau commented Jan 28, 2018

As discussed in #1858, we want to generate a JSON stats object from Webpack, so that folks can use popular tools like webpack-bundle-analyzer without ejecting.

This change adds a new script: build:with-stats. This new script does exactly the same thing as build, but with the added work of generating build/bundle-stats.json, using bfj to create a write stream.

Test plan:

1. check that the build script works

I ran yarn build:with-stats in the root directory of the project. Once it completed, I verified that build/bundle-stats.json existed. I dragged it into Webpack Visualizer to check that the file is correct

2. check that it creates the new script

I ran yarn create-react-app test to generate a new app. I checked that the new script is documented in that initial echo:

screen shot 2018-01-28 at 1 00 06 pm

I ran cd test, and then yarn build:with-stats. Similarly, I verified that the file was created in build/bundle-stats.json.

3. check the instructions for webpack-bundle-analyzer

In the new README, I added steps for using with webpack-bundle-analyzer. I verified that they work by:

  • running yarn add webpack-bundle-analyzer
  • copying this line to package.json: "analyze": "npm run build:with-stats && webpack-bundle-analyzer build/bundle-stats.json",
  • running yarn analyze

After building, it automatically opened a browser window:

screen shot 2018-01-28 at 12 54 11 pm

viankakrisna and others added 23 commits January 14, 2018 11:58
* add experimental babel-plugin-macros support

closes facebook#2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](facebook#2730 (comment))
and it worked well.

* Pin the version
facebook#3419)

* Disables navigateFallback and updates the README

* Typos

* Updated a URL in a comment.
…acebook#3124)

* update jest to 21.0.2 to support watchPathIgnorePatterns configuration

* update jest to 21.1.0

* Try bumping Jest

* Bump babel-jest

* Try to debug weird CI failure

* Remove debug code

* Bump other Jest packages

* ffs

* temp

* Revert "temp"

This reverts commit 62aec9a.
* Update dependencies in react-scripts

* Add first pass of working dependencies for babel-preset-react-app and react-scripts

* Bump more dependency versions

* Adjust more versions and edit fix options

* Restore functionality of old preset

* Disable Uglify in iframe webpack

* Apply prettier

* Re-enable cache in dev and clean deps

* Lock packages and move babel/core to dep in preset

* Bump babel-jest

* Re-enable uglify

* Nest forceAllTransforms correctly in webpack config

* Install babel-core bridge for jest

* Add jest-cli and babel-core bridge to make tests in react-error-overlay pass

* Re-enable transform-dynamic-import

* Add dynamic import syntax support back

* Use new babel in kitchensink

* Transform modules in test

* Revert "Transform modules in test"

This reverts commit 539e46a.

* Attempt fix for ejected tests

* try this

* Add regenerator back

* Bump babel deps to beta.34

* Remove bad files

* Use default when requiring babel transform plugin

* Bump deps

* Try the fix?

* Oopsie

* Remove some weird things

* Run Babel on react-error-overlay tests

* Try fixing kitchensink

* Use new API for codeFrame

* Add missing (?) babelrc

* Maybe this helps?

* Maybe fix mocha

* I shouldn't have deleted this 🤦
* Bump eslint-plugin-jsx-a11y

* Update index.js

* Update index.js

* Update package.json

* Don't use links for non-linky things
* Updating ESlint to ^4.15.0 and adding new rules to config

* remoning style rule and auto fixing breakages from new rules

* Removing implicit-arrow-linebreak style rule

* adding new rule to eslint config project

* updating react scripts eslint version

* Pinning version.

* Changing getter-return to warn

* Update package.json

* Update .eslintrc
* Provide better defaults

* Let babel determine features to compile

* meh

* Remove setting of BABEL_ENV

* Revert "Remove setting of BABEL_ENV"

This reverts commit ee2db70.

* Set browsers to ie9
…/jest higher up the tree (facebook#3771)

* Run real scripts in local development

* Add preflight check warning

* I know what I am doing

* Move preflight check into individual scripts

This ensures we don't try to filter NODE_PATH twice, accidentally removing the now-absolute path.

* Slightly tweak the wording

* Fix lint
* Cheap perf gains

* Whoopsie
* Add restricted globals package

* Use new package in eslint-config

* Add eslint-restricted-globals dependency

* Fixes

* Update dependencies

* Update test and README

* Use jest

* tweaks

* Add lint/test to CI

* Fix lint
* Bump dependencies

* Use a more sensible way to compile error overlay

* Keep old chalk for global CLI
@gaearon
Copy link
Contributor

gaearon commented Jan 28, 2018

I don't really see us adding a new top-level script just for this purpose. A flag, maybe.

@joshwcomeau
Copy link
Contributor Author

joshwcomeau commented Jan 28, 2018

I don't really see us adding a new top-level script just for this purpose. A flag, maybe.

Yeah, I was on the fence with that. My rationale was that it would give the feature greater exposure to existing users (I know for myself I rarely check the docs, so I might not notice that this is added, but I WOULD notice if a new script is in new projects).

But yeah, as I type that out now, it feels like weak reasoning; you could say the same thing for any new feature, and we can't just keep adding scripts.

Can change!

I'm also planning on going through and annotating things. Just currently struggling to get git to cooperate (rebased from master and now a bunch of new commits were added somehow??)

EDIT: Ah, my changes were against master, not next. Fixing...

As discussed in facebook#1858, we'd like to make it easy for users to view
bundle stats without ejecting.

This PR adds a new script that, along with building the site for
production, creates a `build-stats.json` file. This file can be
used with tools like Webpack Bundle Analyzer or Webpack Visualizer,
to help the user optimize their bundles.

Conflicts:
	package.json
	packages/react-scripts/package.json
@joshwcomeau
Copy link
Contributor Author

Ugh I made it worse.

I have to go. Gonna close this PR and reopen when it's sorted.

@gaearon
Copy link
Contributor

gaearon commented Jan 28, 2018

Adding it at a top-level would make sense if we wanted to actively promote this feature. However I doubt we'd want that for something that's webpack-specific. If this was something based on sourcemaps I could see us adding it at the top level and promoting more widely.

@joshwcomeau
Copy link
Contributor Author

joshwcomeau commented Jan 31, 2018

Adding it at a top-level would make sense if we wanted to actively promote this feature. However I doubt we'd want that for something that's webpack-specific. If this was something based on sourcemaps I could see us adding it at the top level and promoting more widely.

Yeah, fair points. The tricky thing is that by far the most popular bundle analysis tools, at least by NPM downloads, are webpack-specific. There was a ton of discussion in #1858, but it wasn't really conclusive.

But yeah, I'm happy to have this just be a flag. Should still be very accessible for folks who check the README.

@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.