-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Provide cjs and esm bundles with information about treeshakability #429
Changes from 1 commit
d7e4665
210a599
61aa749
cb4cd0e
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 |
---|---|---|
|
@@ -9,9 +9,7 @@ node_modules/ | |
.DS_Store | ||
|
||
# generated files | ||
lib/ | ||
dist/ | ||
esm/ | ||
|
||
# generated site | ||
site/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,21 @@ | ||
{ | ||
"dist/react-beautiful-dnd.js": { | ||
"bundled": 362816, | ||
"minified": 135326, | ||
"gzipped": 38147 | ||
}, | ||
"dist/react-beautiful-dnd.min.js": { | ||
"bundled": 331396, | ||
"minified": 125563, | ||
"gzipped": 35116 | ||
}, | ||
"dist/react-beautiful-dnd.umd.js": { | ||
"bundled": 362816, | ||
"minified": 135326, | ||
"gzipped": 38147 | ||
}, | ||
"dist/react-beautiful-dnd.esm.js": { | ||
"bundled": 167329, | ||
"minified": 81581, | ||
"gzipped": 20552, | ||
"treeshaked": { | ||
"rollup": 70346, | ||
"webpack": 71950 | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,26 +15,22 @@ | |
"bugs": { | ||
"url": "https://github.com/atlassian/react-beautiful-dnd/issues" | ||
}, | ||
"main": "lib/index.js", | ||
"module": "esm/index.js", | ||
"main": "dist/react-beautiful-dnd.cjs.js", | ||
"module": "dist/react-beautiful-dnd.esm.js", | ||
"sideEffects": false, | ||
"files": [ | ||
"/lib", | ||
"/dist", | ||
"/esm", | ||
"/src" | ||
], | ||
"scripts": { | ||
"test": "jest --config ./jest.config.js && cross-env SNAPSHOT=check yarn build:dist", | ||
"validate": "yarn run lint && yarn run typecheck", | ||
"lint": "yarn eslint .", | ||
"typecheck": "flow check", | ||
"build": "yarn run build:clean && yarn run build:lib && yarn run build:esm && yarn run build:flow && yarn run build:dist", | ||
"build:clean": "rimraf lib && rimraf esm && rimraf dist", | ||
"build:lib": "cross-env NODE_ENV=cjs babel src -d lib", | ||
"build:esm": "babel src -d esm", | ||
"build": "yarn run build:clean && yarn run build:dist && yarn run build:flow", | ||
"build:clean": "rimraf dist", | ||
"build:dist": "rollup -c", | ||
"build:flow": "echo \"// @flow\n\nexport * from '../src';\" | tee lib/index.js.flow esm/index.js.flow", | ||
"build:flow": "echo \"// @flow\n\nexport * from '../src';\" > dist/react-beautiful-dnd.cjs.js.flow", | ||
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. Is flow okay with this? Do we want it also for the .esm.js file too? 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 don't think it's necessary. Users usually do not specify esm entry directly, but rely on bundler. |
||
"storybook": "start-storybook -p 9002", | ||
"build-storybook": "build-storybook -c .storybook -o site", | ||
"prepublish": "yarn run build && cd website && yarn", | ||
|
@@ -89,7 +85,7 @@ | |
"rollup-plugin-commonjs": "^8.2.6", | ||
"rollup-plugin-node-resolve": "^3.0.2", | ||
"rollup-plugin-replace": "^2.0.0", | ||
"rollup-plugin-size-snapshot": "^0.2.1", | ||
"rollup-plugin-size-snapshot": "^0.3.0", | ||
"rollup-plugin-strip": "^1.1.1", | ||
"rollup-plugin-uglify": "^2.0.1", | ||
"styled-components": "^3.2.0" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,10 +8,14 @@ import replace from 'rollup-plugin-replace'; | |
import strip from 'rollup-plugin-strip'; | ||
import { sizeSnapshot } from 'rollup-plugin-size-snapshot'; | ||
|
||
const pkg = require('./package.json'); | ||
|
||
const input = './src/index.js'; | ||
|
||
const extensions = ['.js', '.jsx']; | ||
|
||
const isExternal = id => !id.startsWith('.') && !id.startsWith('/'); | ||
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. can you please add a comment above this line in the code explaining its purpose? Other than that I think I am good to go! |
||
|
||
const getBabelOptions = () => ({ | ||
exclude: 'node_modules/**', | ||
runtimeHelpers: true, | ||
|
@@ -47,6 +51,36 @@ const getUMDConfig = ({ env, file }) => { | |
}; | ||
|
||
export default [ | ||
getUMDConfig({ env: 'development', file: 'dist/react-beautiful-dnd.js' }), | ||
getUMDConfig({ env: 'development', file: 'dist/react-beautiful-dnd.umd.js' }), | ||
|
||
getUMDConfig({ env: 'production', file: 'dist/react-beautiful-dnd.min.js' }), | ||
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. .umd.min.js? 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. This will break pattern :(
I just would like to see this pattern widespread, so |
||
|
||
{ | ||
input, | ||
output: { | ||
file: pkg.main, | ||
format: 'cjs', | ||
}, | ||
external: isExternal, | ||
plugins: [ | ||
resolve({ extensions }), | ||
babel(getBabelOptions()), | ||
strip({ debugger: true }), | ||
], | ||
}, | ||
|
||
{ | ||
input, | ||
output: { | ||
file: pkg.module, | ||
format: 'es', | ||
}, | ||
external: isExternal, | ||
plugins: [ | ||
resolve({ extensions }), | ||
babel(getBabelOptions()), | ||
strip({ debugger: 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. I am keen to keep console.* statements for the non-minified builds. Can the 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. From non minified umd too? It seems console was always stripped from both umd bundles |
||
sizeSnapshot({ updateSnapshot: !checkSnapshot }), | ||
], | ||
}, | ||
]; |
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.
is it common to do this rather than use a seperate folder?
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.
Dumb question, but cjs = CommonJS? 😊
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 think it is not necessary to create a folder for only one file in it. We have five file in the end which look very nice in one folder.
Yes, esm and cjs are the most popular shorthands.