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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b141add
Fix uglify and update size snapshot
TrySound Jun 28, 2018
396063e
Upgrade to babel7
TrySound Jun 28, 2018
e1233a9
Enable loose mode for class properties
TrySound Jun 28, 2018
73e10ca
Enable loose mode for object rest spread
TrySound Jun 28, 2018
f274892
Provide esm babel helpers for esm bundle
TrySound Jun 28, 2018
66f46b7
Ignore puppeteer test
TrySound Jun 28, 2018
3561f1d
adding babelrc just for the storybook
alexreardon Jun 28, 2018
8b978c2
adding some notes about the storybook setup
alexreardon Jun 28, 2018
8024eab
renaming readme
alexreardon Jun 28, 2018
968f875
Merge branch 'dev' of github.com:atlassian/react-beautiful-dnd into b…
alexreardon Jun 28, 2018
71d6146
Merge branch 'dev' of github.com:atlassian/react-beautiful-dnd into b…
alexreardon Jun 29, 2018
6a308e0
updating lock file
alexreardon Jun 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions .babelrc
View file
Open in desktop

This file was deleted.

16 changes: 16 additions & 0 deletions .babelrc.js
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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
}
}
}
}
23 changes: 11 additions & 12 deletions package.json
View file
Open in desktop
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": "^0.0.14",
"memoize-one": "^3.1.1",
"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
View file
Open in desktop
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