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
21 changes: 0 additions & 21 deletions .babelrc

This file was deleted.

16 changes: 16 additions & 0 deletions .babelrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
module.exports = {
presets: [
'@babel/react',
'@babel/flow',
['@babel/env', { modules: false, loose: true }],
],
plugins: [
['@babel/proposal-class-properties', { loose: true }],
['@babel/proposal-object-rest-spread', { loose: true }],
],
comments: false,
};

if (process.env.NODE_ENV === 'test') {
module.exports.plugins.push('@babel/transform-modules-commonjs');
}
27 changes: 16 additions & 11 deletions .size-snapshot.json
Original file line number Diff line number Diff line change
@@ -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

"minified": 148259,
"gzipped": 42162
"bundled": 356054,
"minified": 134070,
"gzipped": 37695
},
"dist/react-beautiful-dnd.min.js": {
"bundled": 353559,
"minified": 135149,
"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

"bundled": 174113,
"minified": 88585,
"gzipped": 22372,
"bundled": 179908,
"minified": 90873,
"gzipped": 23472,
"treeshaked": {
"rollup": 76698,
"webpack": 78485
"rollup": {
"code": 75679,
"import_statements": 733
},
"webpack": {
"code": 77480
}
}
}
}
12 changes: 12 additions & 0 deletions .storybook/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"presets": [
"@babel/react",
"@babel/flow",
["@babel/env", { "modules": false, "loose": true }]
],
"plugins": [
["@babel/proposal-class-properties", { "loose": true }],
["@babel/proposal-object-rest-spread", { "loose": true }]
],
"comments": false
}
6 changes: 6 additions & 0 deletions .storybook/babel-setup.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Babel

storybook looks for a root `.babelrc` in the project for its babel config. However, we are using `.babelrc.js` which is not supported. Rather than putting effort into this we are just having a custom `.babelrc` in this folder which is the same as `.babelrc.js`. This is lame, but we are looking to move away from storybook in the short term anyway.

- [Storybook babel docs](https://storybook.js.org/configurations/custom-babel-config/)
- [Storybook issue for supporting `babelrc.js`](https://github.com/storybooks/storybook/issues/2582)
23 changes: 11 additions & 12 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"prepublishOnly": "yarn build"
},
"dependencies": {
"babel-runtime": "^6.26.0",
"@babel/runtime": "7.0.0-beta.51",
"css-box-model": "^1.0.0",
"memoize-one": "^4.0.0",
"prop-types": "^15.6.1",
Expand All @@ -58,20 +58,19 @@
},
"devDependencies": {
"@atlaskit/css-reset": "^2.0.6",
"@babel/core": "^7.0.0-beta.51",
"@babel/plugin-proposal-class-properties": "^7.0.0-beta.51",
"@babel/plugin-proposal-object-rest-spread": "^7.0.0-beta.51",
"@babel/plugin-transform-modules-commonjs": "^7.0.0-beta.51",
"@babel/plugin-transform-runtime": "^7.0.0-beta.51",
"@babel/preset-env": "^7.0.0-beta.51",
"@babel/preset-flow": "^7.0.0-beta.51",
"@babel/preset-react": "^7.0.0-beta.51",
"@storybook/react": "^3.4.8",
"babel-cli": "^6.26.0",
"babel-core": "^6.26.3",
"babel-core": "^7.0.0-bridge.0",
"babel-eslint": "^8.2.5",
"babel-jest": "^23.2.0",
"babel-plugin-styled-components": "^1.5.1",
"babel-plugin-transform-class-properties": "^6.24.1",
"babel-plugin-transform-es2015-modules-commonjs": "^6.26.2",
"babel-plugin-transform-export-extensions": "^6.22.0",
"babel-plugin-transform-object-rest-spread": "^6.26.0",
"babel-plugin-transform-runtime": "^6.23.0",
"babel-preset-env": "^1.7.0",
"babel-preset-flow": "^6.23.0",
"babel-preset-react": "^6.24.1",
"cross-env": "^5.2.0",
"enzyme": "^3.3.0",
"enzyme-adapter-react-16": "^1.1.1",
Expand All @@ -95,7 +94,7 @@
"react-test-renderer": "^16.3.1",
"rimraf": "^2.6.2",
"rollup": "^0.61.2",
"rollup-plugin-babel": "^3.0.5",
"rollup-plugin-babel": "^4.0.0-beta.5",
"rollup-plugin-commonjs": "^9.1.3",
"rollup-plugin-node-resolve": "^3.3.0",
"rollup-plugin-replace": "^2.0.0",
Expand Down
21 changes: 12 additions & 9 deletions rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@

import resolve from 'rollup-plugin-node-resolve';
import commonjs from 'rollup-plugin-commonjs';
import uglify from 'rollup-plugin-uglify';
import babel from 'rollup-plugin-babel';
import replace from 'rollup-plugin-replace';
import strip from 'rollup-plugin-strip';
import { uglify } from 'rollup-plugin-uglify';
import { sizeSnapshot } from 'rollup-plugin-size-snapshot';

const pkg = require('./package.json');
import pkg from './package.json';

const input = './src/index.js';
const extensions = ['.js', '.jsx'];
Expand All @@ -17,9 +16,10 @@ const extensions = ['.js', '.jsx'];
// e.g. 'react'
const excludeAllExternals = id => !id.startsWith('.') && !id.startsWith('/');

const getBabelOptions = () => ({
const getBabelOptions = ({ useESModules }) => ({
exclude: 'node_modules/**',
runtimeHelpers: true,
plugins: [['@babel/transform-runtime', { useESModules }]],
});

const matchSnapshot = process.env.SNAPSHOT === 'match';
Expand All @@ -39,7 +39,7 @@ export default [
// Only deep dependency required is React
external: ['react'],
plugins: [
babel(getBabelOptions()),
babel(getBabelOptions({ useESModules: true })),
resolve({ extensions }),
commonjs({ include: 'node_modules/**' }),
replace({ 'process.env.NODE_ENV': JSON.stringify('development') }),
Expand All @@ -60,7 +60,7 @@ export default [
plugins: [
// Setting production env before running babel etc
replace({ 'process.env.NODE_ENV': JSON.stringify('production') }),
babel(getBabelOptions()),
babel(getBabelOptions({ useESModules: true })),
resolve({ extensions }),
commonjs({ include: 'node_modules/**' }),
strip({ debugger: true }),
Expand All @@ -75,18 +75,21 @@ export default [
input,
output: { file: pkg.main, format: 'cjs' },
external: excludeAllExternals,
plugins: [resolve({ extensions }), babel(getBabelOptions())],
plugins: [
resolve({ extensions }),
babel(getBabelOptions({ useESModules: false })),
],
},
// EcmaScript Module (esm) build
// - Keeping console.log statements
// - All external packages are not bundled
{
input,
output: { file: pkg.module, format: 'es' },
output: { file: pkg.module, format: 'esm' },
external: excludeAllExternals,
plugins: [
resolve({ extensions }),
babel(getBabelOptions()),
babel(getBabelOptions({ useESModules: true })),
sizeSnapshot({ matchSnapshot }),
],
},
Expand Down
Loading