From edb446f8a76267f3dc92f42b5e91d4c9ecfa2702 Mon Sep 17 00:00:00 2001 From: Joe Lencioni Date: Thu, 12 Oct 2017 10:15:19 -0700 Subject: [PATCH] Build with rollup instead of webpack Rollup will produce a smaller and more optimized bundle than webpack, and can be configured in a way that works perfectly for libraries, such as Aphrodite. This will help to minimize the bundle size impact of using this package, and may even give a small runtime speedup. For reference, React 16 is built using Rollup. Rollup does not allow Babel 5, so I also updated to Babel 6 at the same time. In this update, I tried to take care to maintain the same list of browser support that we have listed in the CSS prefixes that we build. At some point, we probably want to unify this configuration via browserslist. Issue: https://github.com/Khan/aphrodite/issues/239 I noticed that this Babel update caused branch coverage to drop a little, and I was unable to fix it by adding a test that definitely covered the missing branch. Thankfully, all I needed to do was add the istanbul Babel plugin to fix this. This is the approach recommended by the istanbul documentation: https://github.com/istanbuljs/nyc#use-with-babel-plugin-istanbul-for-babel-support Along with this update, I decided to add an ES modules build since it was easy enough. This will be used automatically by tools such as webpack 2+ to import the ES6 module version directly. I think it still makes sense to run these through Babel since most people don't run their node_modules through Babel, so the main difference here is that the import/export statements will not be compiled to require/module.exports. This allows webpack to perform optimizations such as tree-shaking and scope hoisting. One risk to be on the lookout for when people update to this version is that if you are using `require` to bring in Aphrodite with a version of webpack that is ES modules capable, it will break. Those consumers will need to switch to import instead. For this reason, I would be okay with removing the `module` field from the package.json for an initial release of the rollup build, and then we can add it later when the ecosystem has time to catch up. This is the approach we landed on for react-waypoint: 1. https://github.com/brigade/react-waypoint/pull/220 2. https://github.com/brigade/react-waypoint/pull/223 The filesize of the dist builds before this change looked like: ``` 83K aphrodite.js 84K aphrodite.umd.js 104K aphrodite.umd.js.map 23K aphrodite.umd.min.js 204K aphrodite.umd.min.js.map ``` And after this change: ``` 72K aphrodite.js 73K aphrodite.umd.js 108K aphrodite.umd.js.map 20K aphrodite.umd.min.js 93K aphrodite.umd.min.js.map ``` So it looks like the minified UMD build dropped from 24 KiB to 20 KiB. --- .babelrc | 22 +++++++++ package.json | 28 ++++++----- rollup.config.js | 72 ++++++++++++++++++++++++++++ src/exports.js | 8 ++-- src/index.js | 17 ++++++- src/no-important.js | 19 +++++++- tests/index_test.js | 2 +- tests/ordered-elements_test.js | 88 +++++++++++++++++++++------------- webpack.config.js | 17 ------- 9 files changed, 202 insertions(+), 71 deletions(-) create mode 100644 .babelrc create mode 100644 rollup.config.js delete mode 100644 webpack.config.js diff --git a/.babelrc b/.babelrc new file mode 100644 index 0000000..13c978c --- /dev/null +++ b/.babelrc @@ -0,0 +1,22 @@ +{ + "presets": [["airbnb", { + "modules": false, + "additionalTargets": { + "chrome": "30", + "android": "4", + "firefox": "25", + "ios": "6", + "safari": "6", + "ie": "9", + "edge": "12", + "opera": "13", + }, + }]], + + "env": { + "test": { + "presets": ["airbnb"], + "plugins": ["istanbul"], + }, + }, +} diff --git a/package.json b/package.json index d4e9d97..20c8fc5 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "inline-styles" ], "main": "lib/index.js", + "module": "lib/index.mjs", "scripts": { "lint": "eslint --fix --cache . && flow check", "test": "npm run coverage", @@ -15,20 +16,14 @@ "coverage": "nyc --check-coverage --lines 100 --branches 100 npm run tests", "coveralls": "npm run coverage && nyc report --reporter=text-lcov | coveralls", "pretest": "npm run build:prefixes", - "tests": "mocha --compilers js:babel/register tests", - "tests:watch": "mocha --watch --compilers js:babel/register tests", + "tests": "BABEL_ENV=test mocha --compilers js:babel-register tests", + "tests:watch": "BABEL_ENV=test mocha --watch --compilers js:babel-register tests", "prebuild": "rimraf dist/* lib/*", "build": "npm-run-all --parallel build:*", "watch:build": "npm-run-all --parallel watch:build:*", "build:prefixes": "tools/generate_prefixer_data.js", - "build:main": "babel -d lib/ src", + "build:main": "rollup -c", "watch:build:main": "npm run build:main -- --watch", - "build:umd": "webpack --output-library-target umd --output-library aphrodite --output-filename aphrodite.umd.js --devtool source-map", - "watch:build:umd": "npm run build:umd -- --watch", - "build:umdmin": "webpack --output-library-target umd --output-library aphrodite --output-filename aphrodite.umd.min.js -p --devtool source-map", - "watch:build:umdmin": "npm run build:umdmin -- --watch", - "build:commonjs": "webpack --output-library-target commonjs2 --output-filename aphrodite.js", - "watch:build:commonjs": "npm run build:commonjs -- --watch", "preversion": "npm test", "version": "npm run build && git add dist" }, @@ -43,9 +38,11 @@ }, "homepage": "https://github.com/Khan/aphrodite", "devDependencies": { - "babel": "^5.8.23", - "babel-core": "^5.8.25", - "babel-loader": "^5.3.2", + "babel-cli": "^6.26.0", + "babel-core": "^6.26.0", + "babel-plugin-istanbul": "^4.1.5", + "babel-preset-airbnb": "^2.4.0", + "babel-register": "^6.26.0", "caniuse-api": "^2.0.0", "chai": "^3.3.0", "coveralls": "^2.12.0", @@ -59,7 +56,12 @@ "npm-run-all": "^1.7.0", "nyc": "^6.4.4", "rimraf": "^2.5.2", - "webpack": "^1.12.2" + "rollup": "^0.50.0", + "rollup-plugin-babel": "^3.0.2", + "rollup-plugin-commonjs": "^8.2.1", + "rollup-plugin-node-resolve": "^3.0.0", + "rollup-plugin-replace": "^2.0.0", + "rollup-plugin-uglify": "^2.0.1" }, "dependencies": { "asap": "^2.0.3", diff --git a/rollup.config.js b/rollup.config.js new file mode 100644 index 0000000..3482b9c --- /dev/null +++ b/rollup.config.js @@ -0,0 +1,72 @@ +import babel from 'rollup-plugin-babel'; +import commonjs from 'rollup-plugin-commonjs'; +import replace from 'rollup-plugin-replace'; +import resolve from 'rollup-plugin-node-resolve'; +import uglify from 'rollup-plugin-uglify'; + +import pkg from './package.json'; + +function distBuild(options = {}) { + return { + input: 'src/index.js', + output: { + file: `dist/${options.filename}`, + format: options.format, + name: 'aphrodite', + sourcemap: options.sourcemap, + }, + plugins: [ + babel({ + exclude: ['node_modules/**'], + }), + replace({ + 'process.env.NODE_ENV': JSON.stringify('production'), + }), + resolve({ + browser: true, + }), // so rollup can find node modules + commonjs(), // so rollup can convert node modules to ESM if needed + options.minify && uglify(), + ] + }; +} + +function standardBuilds(filename) { + return { + input: `src/${filename}.js`, + external: [ + ...Object.keys(pkg.dependencies), + 'inline-style-prefixer/static/createPrefixer', + 'inline-style-prefixer/static/plugins/calc', + 'inline-style-prefixer/static/plugins/crossFade', + 'inline-style-prefixer/static/plugins/cursor', + 'inline-style-prefixer/static/plugins/filter', + 'inline-style-prefixer/static/plugins/flex', + 'inline-style-prefixer/static/plugins/flexboxIE', + 'inline-style-prefixer/static/plugins/flexboxOld', + 'inline-style-prefixer/static/plugins/gradient', + 'inline-style-prefixer/static/plugins/imageSet', + 'inline-style-prefixer/static/plugins/position', + 'inline-style-prefixer/static/plugins/sizing', + 'inline-style-prefixer/static/plugins/transition', + ], + output: [ + { file: `lib/${filename}.js`, format: 'cjs' }, + { file: `lib/${filename}.mjs`, format: 'es' }, + ], + plugins: [ + babel({ + exclude: ['node_modules/**'], + }), + commonjs(), // so rollup can convert node modules to ESM if needed + ] + }; +} + +export default [ + distBuild({ filename: 'aphrodite.umd.js', format: 'umd', sourcemap: true, minify: false }), + distBuild({ filename: 'aphrodite.umd.min.js', format: 'umd', sourcemap: true, minify: true }), + distBuild({ filename: 'aphrodite.js', format: 'cjs', sourcemap: false, minify: false }), + standardBuilds('index'), + standardBuilds('no-important'), +]; diff --git a/src/exports.js b/src/exports.js index 33d2180..d9fe9de 100644 --- a/src/exports.js +++ b/src/exports.js @@ -89,10 +89,10 @@ const StyleSheetTestUtils = { * Generate the Aphrodite API exports, with given `selectorHandlers` and * `useImportant` state. */ -const makeExports = ( +export default function makeExports( useImportant /* : boolean */, selectorHandlers /* : SelectorHandler[] */ -) => { +) { return { StyleSheet: { ...StyleSheet, @@ -136,6 +136,4 @@ const makeExports = ( useImportant, styleDefinitions, selectorHandlers); }, }; -}; - -module.exports = makeExports; +} diff --git a/src/index.js b/src/index.js index c3a1d18..c3ba904 100644 --- a/src/index.js +++ b/src/index.js @@ -2,7 +2,22 @@ import {defaultSelectorHandlers} from './generate'; import makeExports from './exports'; const useImportant = true; // Add !important to all style definitions -export default makeExports( + +const Aphrodite = makeExports( useImportant, defaultSelectorHandlers ); + +const { + StyleSheet, + StyleSheetServer, + StyleSheetTestUtils, + css, +} = Aphrodite; + +export { + StyleSheet, + StyleSheetServer, + StyleSheetTestUtils, + css, +}; diff --git a/src/no-important.js b/src/no-important.js index 84be1a3..e77d369 100644 --- a/src/no-important.js +++ b/src/no-important.js @@ -2,11 +2,26 @@ // Module with the same interface as the core aphrodite module, // except that styles injected do not automatically have !important // appended to them. -import {defaultSelectorHandlers} from './generate'; +import { defaultSelectorHandlers } from './generate'; import makeExports from './exports'; const useImportant = false; // Don't add !important to style definitions -export default makeExports( + +const Aphrodite = makeExports( useImportant, defaultSelectorHandlers ); + +const { + StyleSheet, + StyleSheetServer, + StyleSheetTestUtils, + css, +} = Aphrodite; + +export { + StyleSheet, + StyleSheetServer, + StyleSheetTestUtils, + css, +}; diff --git a/tests/index_test.js b/tests/index_test.js index 4069a0d..fd741fc 100644 --- a/tests/index_test.js +++ b/tests/index_test.js @@ -1,5 +1,5 @@ import asap from 'asap'; -import {assert} from 'chai'; +import { assert } from 'chai'; import jsdom from 'jsdom'; import { diff --git a/tests/ordered-elements_test.js b/tests/ordered-elements_test.js index 352b91a..b864c96 100644 --- a/tests/ordered-elements_test.js +++ b/tests/ordered-elements_test.js @@ -2,95 +2,95 @@ import {assert} from 'chai'; import OrderedElements from '../src/ordered-elements'; -describe("OrderedElements", () => { - it("can identify elements it has", () => { +describe('OrderedElements', () => { + it('can identify elements it has', () => { const elems = new OrderedElements(); - elems.set("a", 1); - assert.equal(elems.has("a"), true); + elems.set('a', 1); + assert.equal(elems.has('a'), true); }); - it("can identify elements it does not have", () => { + it('can identify elements it does not have', () => { const elems = new OrderedElements(); - elems.set("a", 1); - assert.equal(elems.has("b"), false); + elems.set('a', 1); + assert.equal(elems.has('b'), false); }); - it("can get elements it has", () => { + it('can get elements it has', () => { const elems = new OrderedElements(); - elems.set("a", 1); - assert.equal(elems.get("a"), 1); + elems.set('a', 1); + assert.equal(elems.get('a'), 1); }); - it("adds new elements in order", () => { + it('adds new elements in order', () => { const elems = new OrderedElements(); - elems.set("a", 1); - elems.set("b", 2); + elems.set('a', 1); + elems.set('b', 2); assert.deepEqual({ elements: { a: 1, b: 2, }, - keyOrder: ["a", "b"], + keyOrder: ['a', 'b'], }, elems); }); it("overrides old elements but doesn't add to the key ordering", () => { const elems = new OrderedElements(); - elems.set("a", 1); - elems.set("a", 2); + elems.set('a', 1); + elems.set('a', 2); assert.deepEqual({ elements: { a: 2, }, - keyOrder: ["a"], + keyOrder: ['a'], }, elems); }); it('preserves original order when overriding', () => { const elems = new OrderedElements(); - elems.set("a", 1); - elems.set("b", 1); - elems.set("a", 2); + elems.set('a', 1); + elems.set('b', 1); + elems.set('a', 2); assert.deepEqual({ elements: { a: 2, b: 1, }, - keyOrder: ["a", "b"], + keyOrder: ['a', 'b'], }, elems); }); it('can reorder when overriding', () => { const elems = new OrderedElements(); - elems.set("a", 1); - elems.set("b", 1); - elems.set("a", 2, true); + elems.set('a', 1); + elems.set('b', 1); + elems.set('a', 2, true); assert.deepEqual({ elements: { b: 1, a: 2, }, - keyOrder: ["b", "a"], + keyOrder: ['b', 'a'], }, elems); }); - it("iterates over the elements in the correct order", () => { + it('iterates over the elements in the correct order', () => { const elems = new OrderedElements(); - elems.set("a", 1); - elems.set("b", 2); - elems.set("c", 3); + elems.set('a', 1); + elems.set('b', 2); + elems.set('c', 3); const order = []; @@ -99,9 +99,33 @@ describe("OrderedElements", () => { }); assert.deepEqual([ - ["a", 1], - ["b", 2], - ["c", 3], + ['a', 1], + ['b', 2], + ['c', 3], ], order); }); + + it('works with nested Maps', () => { + const elems = new OrderedElements(); + elems.set('a', 1); + elems.set('b', new Map([['ba', 1], ['bb', 2]])); + elems.set('c', 3); + + elems.set('b', new Map([['ba', 3]])); + + assert.deepEqual({ + elements: { + a: 1, + b: { + elements: { + ba: 3, + bb: 2, + }, + keyOrder: ['ba', 'bb'], + }, + c: 3, + }, + keyOrder: ['a', 'b', 'c'], + }, elems); + }); }); diff --git a/webpack.config.js b/webpack.config.js deleted file mode 100644 index 7bf8bba..0000000 --- a/webpack.config.js +++ /dev/null @@ -1,17 +0,0 @@ -const path = require('path'); - -module.exports = { - entry: [ - './src/index' - ], - output: { - path: path.join(__dirname, 'dist') - }, - module: { - loaders: [{ - test: /\.js$/, - loaders: ['babel'], - include: path.join(__dirname, 'src') - }] - } -}