Skip to content

Added module entry point (+ refactored build setup) #2

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

Open
Andarist wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added module entry point (+ refactored build setup) #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 1 addition & 5 deletions .babelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
{
"presets": [
"env",
"stage-0",
"react"
]
"presets": ["./.babelrc.js"]
}
11 changes: 11 additions & 0 deletions .babelrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
const { NODE_ENV, BABEL_ENV } = process.env

const modules = BABEL_ENV === 'cjs' || NODE_ENV === 'test' ? 'commonjs' : false

module.exports = {
presets: [
['env', { modules, loose: true }],
'stage-0',
'react'
]
}
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
dist
node_modules
coverage
.nyc_output
coverage.lcov
21 changes: 15 additions & 6 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@
"name": "macro-components",
"version": "1.0.0",
"description": "Create flexible layout and composite UI components without the need to define arbitrary custom props.",
"main": "dist/index.js",
"main": "dist/macro-components.cjs.js",
"module": "dist/macro-components.es.js",
"scripts": {
"prepublish": "babel src -d dist",
"prepublish": "npm run build",
"build": "rollup -c",
"start": "x0 dev docs/App.js -o",
"docs": "x0 build docs/App.js --static --cssLibrary='styled-components' -d docs",
"cover": "nyc report --reporter=html --reporter=lcov > coverage.lcov && codecov",
"bench": "babel-node benchmarks/index.js",
"size": "npm run prepublish && bundlesize",
"test": "nyc ava"
"bench": "cross-env BABEL_ENV=cjs babel-node benchmarks/index.js",
"size": "npm run build && bundlesize",
"test": "cross-env NODE_ENV=test nyc ava"
},
"keywords": [
"react",
Expand All @@ -22,6 +24,7 @@
],
"author": "Brent Jackson",
"license": "MIT",
"sideEffects": false,
"devDependencies": {
"@compositor/x0": "^3.0.4",
"ava": "^0.23.0",
Expand All @@ -33,15 +36,21 @@
"benchmark": "^2.1.4",
"bundlesize": "^0.15.3",
"codecov": "^3.0.0",
"cross-env": "^5.1.3",
"nyc": "^11.2.1",
"prop-types": "^15.6.0",
"react": "^16.0.0",
"react-dom": "^16.0.0",
"react-test-renderer": "^16.0.0",
"rollup": "^0.55.1",
"rollup-plugin-babel": "^3.0.3",
"styled-components": "^2.2.3",
"styled-system": "^1.0.8",
"system-components": "^1.1.1"
},
"peerDependencies": {
"react": "^15.x || ^16.x"
},
"ava": {
"require": [
"babel-register"
Expand All @@ -54,7 +63,7 @@
"maxSize": "0.5 kb"
},
{
"path": "./dist/index.js",
"path": "./dist/macro-components.cjs.js",
"maxSize": "1.5 kb"
}
]
Expand Down
20 changes: 20 additions & 0 deletions rollup.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import babel from 'rollup-plugin-babel'

@jxnblk jxnblk Feb 1, 2018

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Generally I like to keep bundlers/etc. out of libraries. This whole setup feels very heavy-handed; is there not a simpler way to handle es bundles and support tree shaking these days?

@Andarist Andarist Feb 1, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I like to keep bundlers/etc. out of libraries.

I don't really see how it differs from having a babel setup in place (which you did). Advantage of this approach is that it is a single file/script that outputs both module types at once - esm and cjs. And it produces "flat bundles" by default - which is not relevant at the moment as this library is in a single file, but might be relevant in the future if you decide to add more files.

One alternatives is to have 2 build scripts, something along lines:

"build": "npm run build:es && npm run build:cjs",
"build:es": "cross-env BABEL_ENV=es babel src --out-dir es",
"build:cjs": "cross-env BABEL_ENV=cjs babel src --out-dir cjs",

And apply modules transform conditionally in the babelrc (like in this PR). Ain't really sure if this makes things easier.

One great alternative is rising on the horizon - microbundle, which should ease part of this whole setup and hide most things behind its implementation (which is just opinionated wrapper around rollup) and a single CLI command. But at the time being it has no babel support yet (it uses buble), and I don't feel yet like using it, because of this.

Bottom line - if you want to support tree-shaking for esm users and also have cjs entries, so there is no problem when requiring things in tests etc, then you have to transpile somehow conditionally with 2 targets in mind.

@jxnblk jxnblk Feb 4, 2018

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt this library will ever be more than one file. With babel, it simply transpiles a new version of ES syntax to one that's more widely compatible – i.e. it's still just JavaScript, whereas a bundle changes how module syntax works. If this library is published to npm, I'm relying on the CommonJS module, which is how npm works.

I'd rather hold out for a solution from npm and the node community for using the ES import syntax than relying on a bundler and a custom setup that is likely to change, if that makes sense

@Andarist Andarist Feb 4, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i.e. it's still just JavaScript, whereas a bundle changes how module syntax works.

Not sure what do you mean by that, output files are "just JavaScript" - no matter if you use babel alone or a bundler (rollup in this case) for transpilation. Both produce the same CJS output when it comes to module syntax (if you use more than default export, which you do).

If this library is published to npm, I'm relying on the CommonJS module, which is how npm works.

This is not really true, unless you stick to using node or older tooling. If you use webpack@2+ (or other module aware bundler) you are probably already relaying on non-cjs files - if you rely on libraries that have "module" field and there are already a lot of them in the ecosystem (styled-components included).

I'd rather hold out for a solution from npm and the node community for using the ES import syntax than relying on a bundler and a custom setup that is likely to change, if that makes sense

Well, this PR attempts to cover both cases - this is very much usable in node (for SSR etc), node will grab cjs file specified by the "main" field in package.json. Bundlers will be able to pick up the file specified in the "module" field and benefit from it - tree-shake, scope hoist.

It doesnt require a custom setup from user's perspective, and when it comes to library developer - you are already having a custom setup with babel, conceptually it isn't any different from what is presented here.

import pkg from './package.json'

export default {
input: 'src/index.js',
output: [{
file: pkg.module,
format: 'es',
exports: 'named',
}, {
file: pkg.main,
format: 'cjs',
exports: 'named',
}],
external: [
...Object.keys(pkg.dependencies || {}),
...Object.keys(pkg.peerDependencies || {}),
],
plugins: [babel()],
}
2 changes: 1 addition & 1 deletion src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react'
import * as React from 'react'

export const macro = template => {
class Macro extends React.Component {
Expand Down