Turn on Babel `helpers` #5093

Timer · 2018-09-25T20:50:03Z

This turns on Babel helpers to reduce bundle size.

Resolves long standing #238 and recently opened #5092.

Timer · 2018-09-25T20:51:07Z

gaearon · 2018-09-25T21:43:55Z

packages/babel-preset-react-app/create.js

@@ -27,6 +17,7 @@ module.exports = function(api, opts, env) {
  var isEnvProduction = env === 'production';
  var isEnvTest = env === 'test';
  var isFlowEnabled = validateBoolOption('flow', opts.flow, true);
+  var areHelpersEnabled = validateBoolOption('helpers', opts.helpers, true);


Wait, shouldn't default be false?

This is the non-dependency version.

Ah it's for app code. Hmm.

Would it break these instructions?

https://reactjs.org/docs/add-react-to-a-website.html#run-jsx-preprocessor

If so I think it should still be off by default.

See https://github.com/facebook/create-react-app/pull/5093/files#diff-e4eb38a3161bed144100754a3e97763dR26

Right but I don't want this to break non-commonjs usage.

Or maybe we should have a separate entry point for "standalone" usage.

gaearon · 2018-09-25T21:45:13Z

packages/babel-preset-react-app/utils.js

+ */
+'use strict';
+
+const validateBoolOption = (name, value, defaultValue) => {


Copy paste is your friend! Although I don't care

As in you'd rather it be copypasta instead of a util file? 😄

If it's something trivial like this, yeah I don't mind copy pasta.

gaearon · 2018-09-25T21:49:38Z

packages/react-scripts/config/jest/babelTransform.js

@@ -10,6 +10,6 @@
 const babelJest = require('babel-jest');

 module.exports = babelJest.createTransformer({
-  presets: [require.resolve('babel-preset-react-app')],
+  presets: [[require.resolve('babel-preset-react-app'), { helpers: true }]],


Doesn't matter for Jest though?

Probably not, how about eject? This is why I'd rather it by on by default, see eject.js.

gaearon · 2018-09-25T21:51:21Z

packages/react-scripts/config/webpack.config.dev.js

@@ -234,7 +234,12 @@ module.exports = {
                options: {
                  // @remove-on-eject-begin


This is removed on eject. Where do we decide what to write on eject? Seems like we'd lose this. Maybe it means I wasn't right about the app default.

https://github.com/facebook/create-react-app/pull/5093/files#diff-64d71298908161d0a835f0b72170c307R211

gaearon · 2018-09-25T21:55:06Z

If we go ahead we'll need to figure out what to change our standalone JSX instructions to before this hits stable.

Actually it might be sufficient to just disable helpers in /dev and /prod exports, but leave it on in the main one.

Timer · 2018-09-25T21:56:45Z

Hmm, OK. We'll make helpers on by default for apps, helpers off by default for dependencies, and helpers off by default for /dev, /prod.

What about /test?

gaearon

Let's make it on by default for app (any env), off by default for node_modules (any env).

But also let's make it off for /dev, /prod, /test entry points which are probably mostly used by people without config (and possibly without commonjs). /test is a funny one but let's treat it like other standalones.

Timer · 2018-09-25T22:02:21Z

Ready for review, I dropped the e2e changes for tersity and will have a follow up PR.

stale

gaearon · 2018-09-25T22:11:53Z

packages/babel-preset-react-app/dev.js

@@ -9,5 +9,5 @@
 const create = require('./create');

 module.exports = function(api, opts) {
-  return create(api, opts, 'development');
+  return create(api, { helpers: false, ...(opts || {}) }, 'development');


It's fine to spread undefined. I don't think you need || {}

This is object spread though. Is it supported in all envs we target?

look at that. they thought of everything.

gaearon · 2018-09-25T22:12:42Z

tasks/e2e-kitchensink.sh

@@ -146,10 +146,6 @@ PORT=3001 \
  nohup yarn start &>$tmp_server_log &
 grep -q 'You can now view' <(tail -f $tmp_server_log)

-# Before running Mocha, specify that it should use our preset
-# TODO: this is very hacky and we should find some other solution
-echo '{"presets":["react-app"]}' > .babelrc


Bad find, this broke the tests.

gaearon · 2018-09-25T22:56:16Z

tasks/e2e-kitchensink.sh

@@ -146,6 +146,10 @@ PORT=3001 \
  nohup yarn start &>$tmp_server_log &
 grep -q 'You can now view' <(tail -f $tmp_server_log)

+# We haven't ejected yet which means there's no `babel` key
+# in package.json yet
+echo '{"presets":["react-app"]}' > .babelrc


Can we just put it in our jest integration config or something? Why does it have to be at the root?

I dunno what's best, but during eject we add it to package.json. Will file follow up issue.

I just mean that in this case we only need it for integration test itself. So it's unrelated to our app and I'd like it to stay unrelated (if that makes sense)

That makes sense. We'll figure something out.

* Turn on helpers and test importing something with async/await works * Compiling babel runtime breaks itself * Add helpers option to babel plugin with defaults * Make helpers off by default and on in our configuration * Hit eject and e2e * meh * copy'n'paste * change again * Turn off helpers by default in /prod, /dev, /test * oops * Spread undefined * Use object assign not object spread * Put preset in template since it's needed * Fix e2e tests

Turn on helpers and test importing something with async/await works

775387a

Timer added the tag: enhancement label Sep 25, 2018

Timer added this to the 2.0.0 milestone Sep 25, 2018

facebook-github-bot added the CLA Signed label Sep 25, 2018

Timer added 2 commits September 25, 2018 17:13

Compiling babel runtime breaks itself

ca2777f

Add helpers option to babel plugin with defaults

b86f9bd

gaearon reviewed Sep 25, 2018

View reviewed changes

Make helpers off by default and on in our configuration

937d786

gaearon reviewed Sep 25, 2018

View reviewed changes

Timer added 2 commits September 25, 2018 17:51

Hit eject and e2e

b74852a

meh

3cdbbfe

copy'n'paste

3dbc5f8

gaearon previously requested changes Sep 25, 2018

View reviewed changes

Timer added 2 commits September 25, 2018 17:59

change again

2b06e8b

Turn off helpers by default in /prod, /dev, /test

8ef839d

oops

55a605b

gaearon reviewed Sep 25, 2018

View reviewed changes

Timer added 2 commits September 25, 2018 18:13

Spread undefined

071aa4f

Use object assign not object spread

82c82f8

gaearon approved these changes Sep 25, 2018

View reviewed changes

Timer added 2 commits September 25, 2018 18:35

Put preset in template since it's needed

a404acc

Fix e2e tests

5bb2ee2

gaearon reviewed Sep 25, 2018

View reviewed changes

Timer merged commit adfb20c into facebook:next Sep 25, 2018

Timer mentioned this pull request Sep 25, 2018

See what we can do about .babelrc for E2E #5095

Closed

Timer deleted the feature/enable-helpers branch September 26, 2018 00:53

lock bot locked and limited conversation to collaborators Jan 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turn on Babel `helpers` #5093

Turn on Babel `helpers` #5093

Timer commented Sep 25, 2018

Timer commented Sep 25, 2018

gaearon Sep 25, 2018

Timer Sep 25, 2018

gaearon Sep 25, 2018

Timer Sep 25, 2018

gaearon Sep 25, 2018

gaearon Sep 25, 2018

gaearon Sep 25, 2018

Timer Sep 25, 2018

gaearon Sep 25, 2018

gaearon Sep 25, 2018

Timer Sep 25, 2018

gaearon Sep 25, 2018

Timer Sep 25, 2018

gaearon commented Sep 25, 2018

Timer commented Sep 25, 2018 •

edited

Loading

gaearon left a comment •

edited

Loading

Timer commented Sep 25, 2018 •

edited

Loading

gaearon Sep 25, 2018

gaearon Sep 25, 2018

Timer Sep 25, 2018

gaearon Sep 25, 2018

Timer Sep 25, 2018

gaearon Sep 25, 2018

Timer Sep 25, 2018

gaearon Sep 25, 2018

Timer Sep 26, 2018

		@@ -234,7 +234,12 @@ module.exports = {
		options: {
		// @remove-on-eject-begin

Turn on Babel helpers #5093

Turn on Babel helpers #5093

Conversation

Timer commented Sep 25, 2018

Timer commented Sep 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaearon commented Sep 25, 2018

Timer commented Sep 25, 2018 • edited Loading

gaearon left a comment • edited Loading

Choose a reason for hiding this comment

Timer commented Sep 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Turn on Babel `helpers` #5093

Turn on Babel `helpers` #5093

Timer commented Sep 25, 2018 •

edited

Loading

gaearon left a comment •

edited

Loading

Timer commented Sep 25, 2018 •

edited

Loading