[added] Build step for compiling to CommonJS modules #417

fuller · 2017-06-16T05:22:23Z

Ran into an issue today while trying to update to version 2.0 with errors throwing because of the the static property on the class. Would like to avoid running babel against anything in node_modules, so ideally we'd have a transpiled main field and a module field that points to an ES module build.

The following changes are based on the build steps from Redux repo: https://github.com/reactjs/redux/blob/master/package.json

Changes proposed:

Move all main source code under src so that lib and es can be used for the built files
Entry index.js is now an ES export
~~Move create-react-class to dev dependency as it appears to be no longer necessary in the main library~~ Now removed entirely

I've tested this locally with npm link and everything seems to be working as expected.

Note that there are more build changes that we could make as well, but this solves the initial issue I was seeing 😸

Upgrade Path (for changed or removed APIs):

Acceptance Checklist:

All commits have been squashed to one.
The commit message follows the guidelines in CONTRIBUTING.md.
Documentation (README.md) and examples have been updated as needed.
If this is a code change, a spec testing the functionality has been added.
If the commit message has [changed] or [removed], there is an upgrade path above.

Couldn't get this to merge properly so leaving it in both places

fuller · 2017-06-16T05:26:50Z

.babelrc

+  "presets": ["react"],
+  "env": {
+    "commonjs": {
+      "plugins": [


Supposedly the env and non-env configs should merge, but I was seeing the below issue when I tried to consolidate. Any help is appreciated.

Tried this:

{ "presets": ["react"], "plugins": [ "transform-class-properties", "transform-object-rest-spread" ], "env": { "commonjs": { "presets": ["env"] }, "es": { "presets": [ ["env", { "modules": false }] ] } } }

Got this:

I think we can use the old .babelrc to fix v2.0.0. I think we need to use latest instead of stage-2.

latest isn't enough for this. 'stage-2` should work.

Actually after some tests, .babelrc is working as expected, but the main in package.json is pointing to the non-compiled version. Your change to compile from src to lib should fix without to much change (I believe).

coveralls · 2017-06-16T05:44:21Z

Coverage decreased (-13.2%) to 71.429% when pulling 965a2c3 on ajfuller:master into ec6b8e5 on reactjs:master.

fuller · 2017-06-16T05:46:02Z

Will wait to squash commits until I address any review comments

diasbruno · 2017-06-16T12:00:36Z

Awesome, @ajfuller. This seems close to the commit I forgot to port from the old master (9cb8441).
You can use it as reference.

diasbruno · 2017-06-16T12:04:31Z

.babelrc

+  "presets": ["react"],
+  "env": {
+    "commonjs": {
+      "plugins": [


I think we can use the old .babelrc to fix v2.0.0. I think we need to use latest instead of stage-2.

diasbruno · 2017-06-16T12:18:30Z

yarn.lock can be removed as the build pipeline will add it when making the release commit. I'll update the documentation explaining the build and publish pipeline later.

diasbruno · 2017-06-16T12:29:02Z

.babelrc

+  "presets": ["react"],
+  "env": {
+    "commonjs": {
+      "plugins": [


Actually after some tests, .babelrc is working as expected, but the main in package.json is pointing to the non-compiled version. Your change to compile from src to lib should fix without to much change (I believe).

Fix compilation.

diasbruno · 2017-06-16T13:48:15Z

package.json

@@ -55,14 +56,13 @@
    "react-addons-test-utils": "^15.0.0",
    "react-dom": "^15.0.0",
    "rf-release": "0.4.0",
-    "rimraf": "^2.5.4",
+    "rimraf": "^2.6.1",


rimraf dependency can be removed.

diasbruno · 2017-06-16T13:50:11Z

.gitignore

+
+## Built folders
+es
+lib


I think lib can't be ignored. You can remove es.

coveralls · 2017-06-16T13:51:48Z

Coverage increased (+0.2%) to 84.821% when pulling a3e0332 on ajfuller:master into ec6b8e5 on reactjs:master.

fuller · 2017-06-16T13:54:25Z

@diasbruno I can remove, but wouldn't you want the yarn.lock to be up-to-date for development as well? I'll wait for the documentation updates 😄

coveralls · 2017-06-16T13:59:38Z

Coverage increased (+0.2%) to 84.821% when pulling d13e966 on ajfuller:master into ec6b8e5 on reactjs:master.

diasbruno · 2017-06-16T13:59:42Z

The new pipeline will guarantee that what has been changed on package.json will get on the new release.

fuller · 2017-06-16T14:00:23Z

@diasbruno I've merged your PR with mine and addressed the outstanding comments 👍

coveralls · 2017-06-16T14:26:00Z

Coverage increased (+0.2%) to 84.821% when pulling 846e65b on ajfuller:master into ec6b8e5 on reactjs:master.

fuller · 2017-06-16T14:26:41Z

@diasbruno everything is passing CI now 👍

diasbruno · 2017-06-16T14:27:45Z

I think we are ready to ship! Thank you so much @ajfuller.

diasbruno · 2017-06-16T14:31:15Z

Released v2.0.1.

@JonAbrams

This repeats [the work][0] done on [@JonAbrams 's fork][1] of [reactjs/react-modal][2]. However, it replays the work on top of the latest `master` of `reactjs/react-modal`, so it should stop the PropTypes warnings that it has been generating for quite a while now: ``` Warning: Accessing PropTypes via the main React package is deprecated, and will be removed in React v16.0. Use the latest available v15.* prop-types package from npm instead. For info on usage, compatibility, migration and more, see https://fb.me/prop-types-docs ``` This also brings the fork under the Hired GitHub organization, rather than on a former employee's personal GitHub, so that should be good, too. [0]: JonAbrams@53139fa [1]: https://github.com/JonAbrams/react-modal [2]: https://github.com/reactjs/react-modal

Andrew Fuller added 7 commits June 15, 2017 23:39

Move lib to src so we can use lib for build output

b513046

Use ES module for top level export

f74775f

Remove unnecessary react-create-class dependency

90215c4

Transform based on env

3367687

Couldn't get this to merge properly so leaving it in both places

Create es and commonjs separate build steps

f4dae7e

Update yarn.lock

2bf4f60

Remove unnecessary comma

bd7c0c8

fuller commented Jun 16, 2017

View reviewed changes

Additional changes to support move from src to lib

965a2c3

diasbruno mentioned this pull request Jun 16, 2017

Error importing react-modal #418

Closed

diasbruno requested changes Jun 16, 2017

View reviewed changes

diasbruno and others added 4 commits June 16, 2017 09:45

[fixed] use the correct babel presets combination.

1374871

[fixed] added rules to compile on Makefile.

eab214c

[fixed] removing 'es' for now.

2fc1382

Merge pull request #1 from diasbruno/fix/compilation

a3e0332

Fix compilation.

diasbruno requested changes Jun 16, 2017

View reviewed changes

.gitignore Outdated

## Built folders

es

lib

Copy link

Collaborator

diasbruno Jun 16, 2017

Choose a reason for hiding this comment

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

I think lib can't be ignored. You can remove es.

Address review comments

d13e966

fuller changed the title ~~[added] Create separate build step for ES and CommonJS modules~~ [added] Build step for compiling to CommonJS modules Jun 16, 2017

diasbruno approved these changes Jun 16, 2017

View reviewed changes

Andrew Fuller added 2 commits June 16, 2017 09:02

remove yarn.lock per comment

4351936

Update eslint config so it should pass

846e65b

diasbruno merged commit 435ab91 into reactjs:master Jun 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[added] Build step for compiling to CommonJS modules #417

[added] Build step for compiling to CommonJS modules #417

fuller commented Jun 16, 2017 •

edited

fuller Jun 16, 2017

diasbruno Jun 16, 2017

diasbruno Jun 16, 2017 •

edited

diasbruno Jun 16, 2017 •

edited

coveralls commented Jun 16, 2017 •

edited

fuller commented Jun 16, 2017

diasbruno commented Jun 16, 2017 •

edited

diasbruno Jun 16, 2017

diasbruno commented Jun 16, 2017 •

edited

diasbruno Jun 16, 2017 •

edited

diasbruno Jun 16, 2017

diasbruno Jun 16, 2017

coveralls commented Jun 16, 2017 •

edited

fuller commented Jun 16, 2017

coveralls commented Jun 16, 2017 •

edited

diasbruno commented Jun 16, 2017

fuller commented Jun 16, 2017

coveralls commented Jun 16, 2017 •

edited

fuller commented Jun 16, 2017

diasbruno commented Jun 16, 2017

diasbruno commented Jun 16, 2017

[added] Build step for compiling to CommonJS modules #417

[added] Build step for compiling to CommonJS modules #417

Conversation

fuller commented Jun 16, 2017 • edited

fuller Jun 16, 2017

Choose a reason for hiding this comment

diasbruno Jun 16, 2017

Choose a reason for hiding this comment

diasbruno Jun 16, 2017 • edited

Choose a reason for hiding this comment

diasbruno Jun 16, 2017 • edited

Choose a reason for hiding this comment

coveralls commented Jun 16, 2017 • edited

fuller commented Jun 16, 2017

diasbruno commented Jun 16, 2017 • edited

diasbruno Jun 16, 2017

Choose a reason for hiding this comment

diasbruno commented Jun 16, 2017 • edited

diasbruno Jun 16, 2017 • edited

Choose a reason for hiding this comment

diasbruno Jun 16, 2017

Choose a reason for hiding this comment

diasbruno Jun 16, 2017

Choose a reason for hiding this comment

coveralls commented Jun 16, 2017 • edited

fuller commented Jun 16, 2017

coveralls commented Jun 16, 2017 • edited

diasbruno commented Jun 16, 2017

fuller commented Jun 16, 2017

coveralls commented Jun 16, 2017 • edited

fuller commented Jun 16, 2017

diasbruno commented Jun 16, 2017

diasbruno commented Jun 16, 2017

fuller commented Jun 16, 2017 •

edited

diasbruno Jun 16, 2017 •

edited

diasbruno Jun 16, 2017 •

edited

coveralls commented Jun 16, 2017 •

edited

diasbruno commented Jun 16, 2017 •

edited

diasbruno commented Jun 16, 2017 •

edited

diasbruno Jun 16, 2017 •

edited

coveralls commented Jun 16, 2017 •

edited

coveralls commented Jun 16, 2017 •

edited

coveralls commented Jun 16, 2017 •

edited