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

Adds module and bundle type metadata to the rollup results json #11914

Merged
merged 1 commit into from Dec 23, 2017

Conversation

orta
Copy link
Contributor

@orta orta commented Dec 22, 2017

For #11865 - @gaearon and I discussed the idea of having smaller tables which were grouped by the module name. I had a think about how to do this with the current JSON but it currently looks like this:

{
  "bundleSizes": {
    "react.development.js (UMD_DEV)": {
      "size": 55220,
      "gzip": 14986
    },
    "react.production.min.js (UMD_PROD)": {
      "size": 6546,
      "gzip": 2789
    },
    "react.development.js (NODE_DEV)": {
      "size": 45636,
      "gzip": 12675
    },
    "react.production.min.js (NODE_PROD)": {
      "size": 5444,
      "gzip": 2373
    },
    "React-dev.js (FB_DEV)": {
      "size": 44974,
      "gzip": 12197
    },
    "React-prod.js (FB_PROD)": {
      "size": 12746,
      "gzip": 3440
    },
  • I could grab the environment out of the name, but it'd be brittle.
  • I could grab the module name from the string also, with some real funky string normalization, but that'd be even more brittle.

So instead I updated the results generator to include the actual package name and the environment it was built in. Making it more future proofed.

Made as a separate PR so that the PR #11865 can reference this in master.

@orta orta changed the title [Dev] Adds module and bundle type metadata to the rollup results json Adds module and bundle type metadata to the rollup results json Dec 22, 2017
@@ -1,232 +1,346 @@
{
"bundleSizes": {
"react.development.js (UMD_DEV)": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's just make the whole thing an array and add filename as one of the fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep 👍

@orta orta force-pushed the add_metadata_to_results branch 5 times, most recently from 61c7ba8 to e9513bf Compare December 23, 2017 16:26
@orta
Copy link
Contributor Author

orta commented Dec 23, 2017

Should I have the code handle both array types and object types?

for example http://react.zpao.com/builds/master/latest/results.json is still the older format, which is pulled in an used as the previous result

@gaearon
Copy link
Collaborator

gaearon commented Dec 23, 2017

Just array is fine. After you run yarn prettier and commit, CI won't fail so I can merge this, and then you'll see the array in master too.

@orta
Copy link
Contributor Author

orta commented Dec 23, 2017

👍 cool cool, shipped

@gaearon
Copy link
Collaborator

gaearon commented Dec 23, 2017

OK, this fails CI but probably because master is not updated to use this format. So merging this should actually fix it..

@gaearon gaearon merged commit cf96d84 into facebook:master Dec 23, 2017
@gaearon
Copy link
Collaborator

gaearon commented Dec 24, 2017

There were a few more issues but the master is green now so we should be good!

@orta
Copy link
Contributor Author

orta commented Dec 24, 2017 via email

@gaearon
Copy link
Collaborator

gaearon commented Dec 24, 2017

Not really, it's just scripts :-)

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.

None yet

3 participants