Skip to content

Add ES-modules build #175

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

Merged

Conversation

goto-bus-stop
Copy link
Contributor

Hi 👋 Thanks for y'all's work on redux-actions, it's very useful :)

I do have a small proposal: to publish an ES-modules build as well, for bundlers like Rollup and Webpack2.

This patch conditionally enables the transform-es2015-modules-commonjs transform based on an env var. All the environment setup in the npm scripts is getting a little messy this way, though, because the transform needs to be enabled in all cases except one. I don't think there's a way to conditionally disable a transform in a .babelrc unfortunately :(


What I could do instead is add a "custom" preset that determines whether to set the modules property based on an env var:

// build/preset.js
const esModules = process.env.BABEL_ENV === 'es'
module.exports = {
  presets: [
    ['es2015', { modules: esModules ? false : 'commonjs' }]
  ]
}
// .babelrc
{
  "presets": [
    "./build/preset",
    "stage-0"
  ]
}

That would keep the npm scripts more clean. If that sounds better, let me know!

Publish ES-modules in an es/ folder. Works well with bundlers like
Rollup and Webpack2 :)
@yangmillstheory
Copy link
Contributor

👋

Thanks. I'm out today, but can take a look tonight 👍.

@yangmillstheory
Copy link
Contributor

Wait, could you illustrate what problem this is trying to solve?

I use ES2015 modules with Webpack, so I don't think I've run into the use case.

@goto-bus-stop
Copy link
Contributor Author

It already works fine with Rollup and Webpack2, but they can't apply tree-shaking to the CommonJS module. Also, there's duplicate module interop code: Babel's requireInterop code, and then Rollup or another bundler's glue code to make CommonJS work with their preferred format.

With this PR, bundlers that support ES modules can tree-shake: if Rollup builds a project that is eg. not using the createActions method anywhere, it won't be added to the bundle (and neither will the lodash methods that createActions depends on). And module bundlers that don't support ES modules natively will just keep using the CommonJS build.

@yangmillstheory
Copy link
Contributor

Can we try the build/preset.js approach, since it seems like it'd be cleaner?

@goto-bus-stop
Copy link
Contributor Author

Sure! Added :)

@@ -3,20 +3,24 @@
"version": "1.1.0",
"description": "Flux Standard Action utlities for Redux",
"main": "lib/index.js",
"module": "es/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this module key for? Couldn't find anything about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

module serves about the same purpose as jsnext:main and is used by webpack2 and Rollup. I went by this Rollup wiki page which says

If in doubt, use both!

@@ -0,0 +1,6 @@
const esModules = process.env.BABEL_ENV === 'es';
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like commonjs is the default, so how about

const esOptions = process.env.BABEL_ENV === 'es' ? { modules: false } : {}

That way we don't have to hardcode commonjs and we can just use Babel defaults.

"lint": "esw src webpack.config --color",
"build": "npm run clean && npm run build:es && npm run build:commonjs && npm run build:umd && npm run build:umd:min",
"clean": "rimraf lib es",
"lint": "esw build src webpack.config --color",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we linting build/? Is there even a build/ directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's for the new build/preset-es2015.js file.

@yangmillstheory
Copy link
Contributor

I wonder if it's worth including a comment (or modifying the existing) in this section of the README on the ES2015 build. Thoughts?

@yangmillstheory yangmillstheory merged commit 57e6b64 into redux-utilities:master Dec 13, 2016
@yangmillstheory
Copy link
Contributor

Thanks @goto-bus-stop. I'll publish v1.2.0 shortly.

@yangmillstheory
Copy link
Contributor

https://github.com/acdlite/redux-actions/releases/tag/v1.2.0 has been published.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants