-
Notifications
You must be signed in to change notification settings - Fork 102
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
Use rollup for more compact build output #81
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,28 @@ | ||
{ | ||
"presets": [ | ||
"react-native" | ||
"plugins": [ | ||
"syntax-async-functions", | ||
"syntax-class-properties", | ||
"syntax-trailing-function-commas", | ||
"transform-class-properties", | ||
"transform-es2015-function-name", | ||
"transform-es2015-arrow-functions", | ||
"transform-es2015-block-scoping", | ||
["transform-es2015-classes", { "loose": true }], | ||
"transform-es2015-computed-properties", | ||
"check-es2015-constants", | ||
"transform-es2015-destructuring", | ||
"transform-es2015-parameters", | ||
"transform-es2015-shorthand-properties", | ||
"transform-es2015-spread", | ||
"transform-es2015-template-literals", | ||
"transform-es2015-literals", | ||
"transform-flow-strip-types", | ||
"transform-object-assign", | ||
"transform-object-rest-spread", | ||
"transform-react-display-name", | ||
"transform-react-jsx", | ||
"transform-regenerator", | ||
["transform-es2015-for-of", { "loose": true }], | ||
"external-helpers" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like you are bringing in external helpers now, which means that folks will need to import the external helpers file in order for this to continue working for them. This is almost certainly a breaking change, and I'm hesitant to recommend this approach for a library at this time. Some people are discussing a modular approach to this, so that might become a good option in the future. More info: babel/babel#5699 |
||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ node_modules | |
.node_repl_history | ||
|
||
/lib | ||
/dist | ||
.DS_Store | ||
|
||
examples/interactive-docs/example.bundle.js |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,8 +6,7 @@ | |
"scripts": { | ||
"prepublish": "npm run build:lib", | ||
"build": "npm run build:lib && npm run build:interactive-docs", | ||
"build:lib": "babel src --out-dir lib", | ||
"build:watch": "npm run build:lib -- --watch", | ||
"build:lib": "rollup -c", | ||
"build:interactive-docs": "browserify -e examples/interactive-docs/example.js -o examples/interactive-docs/example.bundle.js", | ||
"test": "echo \"Error: no test specified\" && exit 1" | ||
}, | ||
|
@@ -38,6 +37,7 @@ | |
"devDependencies": { | ||
"babel": "^6.5.2", | ||
"babel-cli": "^6.5.1", | ||
"babel-plugin-external-helpers": "^6.22.0", | ||
"babel-plugin-syntax-object-rest-spread": "^6.5.0", | ||
"babel-plugin-syntax-trailing-function-commas": "^6.5.0", | ||
"babel-plugin-transform-flow-strip-types": "^6.5.0", | ||
|
@@ -46,6 +46,12 @@ | |
"babel-preset-react-native": "^1.4.0", | ||
"browserify": "^13.0.0", | ||
"react": "^15.4.0", | ||
"react-dom": "^15.4.0" | ||
"react-dom": "^15.4.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be 16 now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could, but then the interactive docs will need some refactoring (they use createClass right now). There's nothing about animated that's specific to 15 or 16, so it doesn't need to be changed, but we should move to 16 soon. I want to spend some time integrating animated w/ fiber's scheduling, and so at that time i'll definitely up the dev dep. don't think it should be in this PR though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah gotcha, I'll refactor those and make them not just random inline code |
||
"rollup": "^0.51.5", | ||
"rollup-plugin-babel": "^3.0.2", | ||
"rollup-plugin-commonjs": "^8.2.6", | ||
"rollup-plugin-node-resolve": "^3.0.0", | ||
"rollup-plugin-replace": "^2.0.0", | ||
"rollup-plugin-uglify": "^2.0.1" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
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: 'animated', | ||
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), | ||
...Object.keys(pkg.peerDependencies) | ||
], | ||
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: 'animated.umd.js', format: 'umd', sourcemap: true, minify: false }), | ||
distBuild({ filename: 'animated.umd.min.js', format: 'umd', sourcemap: true, minify: true }), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you already publishing UMD builds? If not, I'm not really sure how much value it adds to start publishing them now, especially if nobody is asking for them. I'd probably leave this out for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed |
||
distBuild({ filename: 'animated.js', format: 'cjs', sourcemap: false, minify: false }), | ||
standardBuilds('index'), | ||
standardBuilds('targets/react-dom'), | ||
standardBuilds('targets/react-native'), | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,13 +10,16 @@ | |
*/ | ||
'use strict'; | ||
|
||
var AnimatedWithChildren = require('./AnimatedWithChildren'); | ||
var InteractionManager = require('./injectable/InteractionManager'); | ||
var AnimatedInterpolation = require('./AnimatedInterpolation'); | ||
var Interpolation = require('./Interpolation'); | ||
var Animation = require('./Animation'); | ||
var guid = require('./guid'); | ||
var Set = global.Set || require('./SetPolyfill'); | ||
import AnimatedWithChildren from './AnimatedWithChildren'; | ||
import InteractionManager from './injectable/InteractionManager'; | ||
import AnimatedInterpolation from './AnimatedInterpolation'; | ||
import Interpolation from './Interpolation'; | ||
import Animation from './Animation'; | ||
import guid from './guid'; | ||
import SetPilyfill from './SetPolyfill'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo: Pilyfill |
||
|
||
// TODO: wonder if we should do the set polyfill another way... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice if the @ljharb might have some thoughts about how to do this best. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it’s way simpler for a standard polyfillable feature like Set (or Map, or Promise, etc) to just assume it’s present, and state in the docs that a polyfill is needed. |
||
var Set = global.Set || SetPilyfill; | ||
|
||
import type { EndCallback } from './Animation'; | ||
import type { InterpolationConfigType } from './Interpolation'; | ||
|
@@ -209,4 +212,4 @@ class AnimatedValue extends AnimatedWithChildren { | |
} | ||
} | ||
|
||
module.exports = AnimatedValue; | ||
export default AnimatedValue; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,4 +38,4 @@ class Animation { | |
} | ||
} | ||
|
||
module.exports = Animation; | ||
export default Animation; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if babel-preset-env can help simplify this config a little?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or babel-preset-airbnb which uses that :-)