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

Babel7 #590

Merged
merged 12 commits into from
Jun 29, 2018
Merged

Babel7 #590

merged 12 commits into from
Jun 29, 2018

Conversation

TrySound
Copy link
Contributor

Ref #525

esm bundle is slightly increased but as we can see user bundle in the end will be smaller.

@TrySound
Copy link
Contributor Author

Hm.. now everything became smaller. First commit should be starting point in diff.

@alexreardon
Copy link
Collaborator

alexreardon commented Jun 28, 2018 via email

Copy link
Collaborator

@alexreardon alexreardon left a comment

Choose a reason for hiding this comment

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

33 down to 23 is great!

@TrySound
Copy link
Contributor Author

TrySound commented Jun 28, 2018

Nope. I didn't strip anything. Just played with babel configuration.

@TrySound
Copy link
Contributor Author

Let's strip invariant in the next PR.

@alexreardon
Copy link
Collaborator

alexreardon commented Jun 28, 2018 via email

@alexreardon
Copy link
Collaborator

storybook only looks for a root .babelrc and not a .babelrc.js. We will be moving away from storybook soon so i went for a super simple option: adding a custom .babelrc to the .storybook folder which is just a match of our .babelrc.js

@@ -1,21 +1,26 @@
{
"dist/react-beautiful-dnd.js": {
"bundled": 391074,
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this the diff on the dev branch on from master?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please let me know:

  • what the change in numbers is between the current dev and this branch? Ie how many kbs do we loose moving to babel 7
  • could you also compare this again the current master? I will put these figures in the release notes

Copy link
Collaborator

Choose a reason for hiding this comment

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

❤️!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can compare the last commit against the first commit in this PR. The first one is only snapshot update.

{
  "dist/react-beautiful-dnd.js": {
-    "bundled": 380027,
-    "minified": 143820,
-    "gzipped": 40855
+    "bundled": 356054,
+    "minified": 134070,
+    "gzipped": 37695
  },
  "dist/react-beautiful-dnd.min.js": {
-    "bundled": 343989,
-    "minified": 130858,
-    "gzipped": 36610
+    "bundled": 320016,
+    "minified": 121108,
+    "gzipped": 33429
  },
  "dist/react-beautiful-dnd.esm.js": {
-    "bundled": 180665,
-    "minified": 91821,
-    "gzipped": 23548,
+    "bundled": 179908,
+    "minified": 90873,
+    "gzipped": 23472,
    "treeshaked": {
      "rollup": {
-        "code": 76262,
-        "import_statements": 874
+        "code": 75679,
+        "import_statements": 733
      },
      "webpack": {
-        "code": 78237
+        "code": 77480
      }
    }
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's against master

{
  "dist/react-beautiful-dnd.js": {
-    "bundled": 400269,
-    "minified": 151652,
-    "gzipped": 43139
+    "bundled": 356054,
+    "minified": 134070,
+    "gzipped": 37695
  },
  "dist/react-beautiful-dnd.min.js": {
-    "bundled": 358556,
-    "minified": 134751,
-    "gzipped": 38201
+    "bundled": 320016,
+    "minified": 121108,
+    "gzipped": 33429
  },
  "dist/react-beautiful-dnd.esm.js": {
-    "bundled": 177759,
-    "minified": 89807,
-    "gzipped": 22649,
+    "bundled": 179908,
+    "minified": 90873,
+    "gzipped": 23472,
    "treeshaked": {
-      "rollup": 78288,
+      "rollup": {
+        "code": 75679,
+        "import_statements": 733
+      },
-      "webpack": 80060
+      "webpack": {
+        "code": 77480
+      }
    }
  }
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

just so i understand - if a user has no shared deps with the library then they take the 179kb bundle size. If they already have all the deps they get 75kb. Is that correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not quite. Bundled, minified, gzipped are sizes of what we provide in rollup config + terser + gzip.

So in production the library size with dependencies will be 121KB to parse and 33KB to load (minified umd).

If user already have all dependencies then the size will be 90KB to parse and 23KB to load subtract development stuff like process.env.NODE_ENV (size snapshot provides production replacement only for treeshaked size)

Treeshaked size doesn't make a lot of sense for this library because as you said there is nothing unused.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be interesting to know what the lowest possible size gzip could be. Ie: if with process.env.NODE_ENV === production and with all dependencies de duplicated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't need to do this here though!

Copy link
Collaborator

@alexreardon alexreardon Jun 29, 2018

Choose a reason for hiding this comment

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

I am still a confused by this:

"treeshaked": {
-      "rollup": 78288,
+      "rollup": {
+        "code": 75679,
+        "import_statements": 733
+      },
-      "webpack": 80060
+      "webpack": {
+        "code": 77480
+      }
    }

And how it relates to the other numbers

"gzipped": 37985
"bundled": 320016,
"minified": 121108,
"gzipped": 33429
},
"dist/react-beautiful-dnd.esm.js": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you please explain the numbers in this section a little bit more? I am a bit confused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

treeshaked code means bundle with this entry point import {} from 'library' which is nothing imported. The result is treeshakability, how much code is left after removing unused code.

import_statements is how many bytes imports of externals takes in the code. It's useful when you want to achieve full treeshakability so other libraries could use this one without worrying about user bundle bloat.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the numbers in the tree shaked section bigger than the UMD builds? I would have thought the UMD would be far bigger as it includes the dependencies...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Treeshaked size is not gzipped. If you will see at minified sizes they are all bigger than treeshaked one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see

Copy link
Collaborator

Choose a reason for hiding this comment

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

So without being able to dedupe anything the esm cost is 23472kb?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes + vendors.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Vendors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

23472kb is the size without vendors and without production replacements

@alexreardon
Copy link
Collaborator

alexreardon commented Jun 29, 2018 via email

@TrySound
Copy link
Contributor Author

Yes. Everything in node_modules

@alexreardon
Copy link
Collaborator

alexreardon commented Jun 29, 2018 via email

@TrySound
Copy link
Contributor Author

Not the same

production umd
minified: 121108
gzipped: 33429

esm
minified: 90873
gzipped: 23472

@alexreardon
Copy link
Collaborator

This is a great win from this PR:

"dist/react-beautiful-dnd.min.js": {
-    "bundled": 343989,
-    "minified": 130858,
-    "gzipped": 36610
+    "bundled": 320016,
+    "minified": 121108,
+    "gzipped": 33429
  },

3kb in the gzip for moving to the new babel. niace

@alexreardon
Copy link
Collaborator

Well done @TrySound !!!!

@alexreardon alexreardon merged commit 33c616e into atlassian:dev Jun 29, 2018
@TrySound TrySound deleted the babel7 branch June 29, 2018 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants