Added jsx-self babel transform plugin #3540

jimfb · 2016-06-20T23:27:53Z

This adds a new transform which appends the __self prop to jsx elements. The transform is needed by one of the React warnings that is being introduced, so users should enable it when running React in DEV mode.

jimfb · 2016-06-20T23:29:23Z

cc @hzoo @kittens We should probably enable this by default (I think it's safe), but only in DEV mode. Do I need to add it to some presets file, or how does that work?

jimfb · 2016-06-20T23:31:47Z

also cc @sebmarkbage

hzoo · 2016-06-20T23:32:03Z

If you mean default to the react-preset yeah, actually already some code there that was commented out (due to http://phabricator.babeljs.io/T2994) doing a similar thing

https://github.com/babel/babel/blob/master/packages/babel-preset-react/index.js

codecov-io · 2016-06-20T23:39:21Z

Current coverage is 88.04%

Merging #3540 into master will decrease coverage by 0.06%

@@             master      #3540   diff @@
==========================================
  Files           193        194     +1   
  Lines         10072       9746   -326   
  Methods        1553       1510    -43   
  Messages          0          0          
  Branches       2199       2106    -93   
==========================================
- Hits           8875       8581   -294   
+ Misses         1197       1165    -32   
  Partials          0          0

Powered by Codecov. Last updated by d0edc1c...88495bf

hzoo · 2016-06-20T23:42:24Z

@jimfb Also if you want, you can just push commits and we can squash into 1 commit at the end.

jimfb · 2016-06-21T00:01:09Z

@hzoo The CI appears to be failing with Error: Cannot find module 'babel-plugin-transform-react-jsx-self'. Is this normal / to be expected for new modules, or is there still an issue on my end? Anything else I need to do on my end?

hzoo · 2016-06-21T00:03:41Z

I see, it's because those tests are using the preset which now has the new module and I don't think lerna 1.x accounts for that. Not sure we can do anything other than switching to lerna 2.x which (I think) fixes it (#3509) or of course releasing/publishing it

jimfb · 2016-06-21T01:44:55Z

@hzoo @kittens Ok, I'll assume the ball is in your court unless I hear otherwise.

hzoo · 2016-06-22T14:05:52Z

packages/babel-plugin-transform-react-jsx-self/src/index.js

+    JSXOpeningElement(node) {
+      const id = t.jSXIdentifier(TRACE_ID);
+      const trace = t.identifier("this");
+      node.container.openingElement.attributes.push(t.jSXAttribute(id, t.jSXExpressionContainer(trace)));


Also is it ok that there's no check for an existing __self attribute?

Should be. The JSX transform will just output a object with multiple __self properties which is fine.

hzoo · 2016-06-22T15:01:00Z

@jimfb Ohh.. I checked the PR out locally and was still confused - it's because we need to add the plugin to the package.json not just the babelrc file haha.

hzoo · 2016-06-22T15:02:42Z

packages/babel-plugin-transform-react-jsx-self/package.json

@@ -0,0 +1,18 @@
+{
+  "name": "babel-plugin-transform-react-jsx-self",
+  "version": "6.0.14",


Not sure if this matters but I would change this to be the current version so 6.0.14

hzoo · 2016-06-22T16:15:05Z

Added in 93299d9

sophiebits · 2016-06-27T07:36:03Z

Didn't we decide last time against including dev-specific stuff in the dev environment for the preset because most people don't set BABEL_ENV (or NODE_ENV) properly during build time?

sebmarkbage · 2016-06-27T08:01:43Z

Yea, this should not go into any preset that the source transform isn't in.

hzoo · 2016-06-27T12:19:36Z

Ah sorry should of checked with someone else? I don't remember if we decided on that but I found http://phabricator.babeljs.io/T2994 which was regarding purerendermixin (although not sure how relevant since it's only in dev? Hmm it must be because it defaults to dev and like mentioned people don't set the env to prod.

Made #3552 to remove it from the preset

Refer: babel/babel#3540

jmm · 2016-06-27T23:20:32Z

@hzoo decided is a strong word, but there was some discussion about it (not between maintainers) starting here: #2963 (comment). I'm not sure I'm convinced that people are going to be oblivious to setting NODE_ENV, as often they already need to do that for other things, like bundling modules properly (React in particular), running server frameworks properly, etc. Not sure it's a big deal to need to configure this stuff explicitly rather than via the preset either, but it'll need good documentation somewhere for people to be aware of it.

jimfb · 2016-06-27T23:27:58Z

Didn't we decide last time against including dev-specific stuff in the dev environment for the preset because most people don't set BABEL_ENV (or NODE_ENV) properly during build time?

I don't remember that being the decision. We backed out the __source transform from the default dev ENV because it broke the older versions of React which didn't have __source whitelisted yet. This change doesn't have the same concern since __self was whitelisted a long time ago, so it is "safer". We could probably also turn on the __source transform at this point. Anyway, that was my recollection, but maybe I'm mistaken, it was a long time ago.

sophiebits · 2016-06-27T23:34:55Z

like bundling modules properly (React in particular), running server frameworks properly

I agree that people probably usually set NODE_ENV properly for runtime, but the question here is what NODE_ENV is when Babel is run, and I don't think most people set that. I know I usually don't.

jmm · 2016-06-28T01:32:05Z

I'm not sure about the details of why jsx-source was removed from the preset. I noticed that it was blowing up when using the API to compile without the filename option, but that was getting fixed or got fixed. To the best of my recollection I saw a comment along the lines of "it's causing a number of problems" and I thought it was described as being removed until problems could be resolved, and I was under the impression that it being commented out was a reflection of that. I'd have to track down the issues / commits to be sure though.

but the question here is what NODE_ENV is when Babel is run, and I don't think most people set that. I know I usually don't.

I don't know if there's any way to get a meaningful amount of information about this kind of usage, but I would guess that a lot of people are compiling with Babel as part of bundling with Browserify or Webpack and setting NODE_ENV. How is this fundamentally different from needing to know that you need to set NODE_ENV when bundling React (for example)? Because if you bundle React for the wrong environment you might notice it via console warnings whereas this would be silent?

Can you help me out with some context and summarize how you're utilizing Babel when you're not setting NODE_ENV? E.g. CLI, API, Browserify + Babelify, Webpack + babel-loader, gulp-babel, some other plugin.

If we need to we can do more in the docs about guiding people to specify the environment.

sophiebits · 2016-06-28T01:37:58Z

In webpack, you would typically use new DefinePlugin({'process.env': {NODE_ENV: JSON.stringify('production')}}), not set NODE_ENV in the build environment. Right? I guess if you are using envify then it's more likely.

jmm · 2016-06-30T02:51:52Z

@spicyj Well, I realize there are a lot of work flows and tools I don't know about, that's why I wanted to better understand your scenario. I often use Babel with Browserify via Babelify, doing something like this for example:

./some-build-script --watch
# or
NODE_ENV=production ./some-build-script

If I'm bundling React with Browserify, envify (loose-envify) is running under the covers automatically and I'm setting the env for it as above.

If a user is just compiling some stuff with Babel and they don't have custom per-env configuration setup, I could see how they'd overlook setting the env or not realize it'd make a difference in that case (which maybe it hasn't up til now). Like I mentioned, if it is important (e.g. if stuff in Babel core is going to behave differently per-env) we could do more with the docs to educate people about that.

In webpack, you would typically use new DefinePlugin({'process.env': {NODE_ENV: JSON.stringify('production')}}), not set NODE_ENV in the build environment. Right?

The way that's illustrated makes it seem static -- in that scenario doesn't the user still need to dynamically set the NODE_ENV passed to Webpack somewhere from the shell or JS?

I don't know if your question was rhetorical, but if we want more perspective on Webpack usage I think @loganfsmyth, for one, knows way more about it than I do. Or I could ask on Slack -- I think a lot of Babel users use Webpack.

Here's one example I found (Redux) where they're doing this (interestingly setting both BABEL_ENV and NODE_ENV, to different values):

cross-env BABEL_ENV=commonjs NODE_ENV=development webpack src/index.js dist/redux.js

And passing that value to Webpack:

var env = process.env.NODE_ENV
// [...]
    new webpack.DefinePlugin({
      'process.env.NODE_ENV': JSON.stringify(env)
    })

(Granted, people working on Redux have sophisticated development skills.)

hzoo added the PR: New Feature 🚀 A type of pull request used for our changelog categories label Jun 20, 2016

hzoo added this to the Next Minor milestone Jun 20, 2016

jimfb force-pushed the jsx-self branch from 88495bf to 7cd3057 Compare June 20, 2016 23:39

jimfb mentioned this pull request Jun 20, 2016

Warn for string refs where owner != __self facebook/react#7091

Closed

hzoo reviewed Jun 22, 2016
View reviewed changes

Added jsx-self babel transform plugin

7d0c4ec

jimfb force-pushed the jsx-self branch from 7cd3057 to 7d0c4ec Compare June 22, 2016 16:06

sebmck merged commit 9229225 into babel:master Jun 22, 2016

SpiritBreaker226 pushed a commit to frig-js/frigging-bootstrap that referenced this pull request Jun 27, 2016

Update babel-preset-react To Version 6.11.0 (#94)

18bd951

Refer: babel/babel#3540

SpiritBreaker226 pushed a commit to frig-js/frig that referenced this pull request Jun 27, 2016

Update babel-preset-react To Version 6.11.0 (#168)

f602bb0

Refer: babel/babel#3540

hzoo modified the milestone: Next Minor Jul 16, 2016

MoOx mentioned this pull request Oct 10, 2016

Make babel-preset-react take an option for dev plugins #4702

Closed

kesne mentioned this pull request Feb 23, 2017

Adding development plugins and using builtins for jsx spread airbnb/babel-preset-airbnb#9

Closed

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 7, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added jsx-self babel transform plugin #3540

Added jsx-self babel transform plugin #3540

jimfb commented Jun 20, 2016

jimfb commented Jun 20, 2016

jimfb commented Jun 20, 2016

hzoo commented Jun 20, 2016 •

edited

Loading

codecov-io commented Jun 20, 2016 •

edited

Loading

hzoo commented Jun 20, 2016

jimfb commented Jun 21, 2016

hzoo commented Jun 21, 2016 •

edited

Loading

jimfb commented Jun 21, 2016

hzoo Jun 22, 2016

sebmck Jun 22, 2016

hzoo commented Jun 22, 2016

hzoo Jun 22, 2016

hzoo commented Jun 22, 2016

sophiebits commented Jun 27, 2016

sebmarkbage commented Jun 27, 2016

hzoo commented Jun 27, 2016 •

edited

Loading

jmm commented Jun 27, 2016

jimfb commented Jun 27, 2016

sophiebits commented Jun 27, 2016

jmm commented Jun 28, 2016

sophiebits commented Jun 28, 2016

jmm commented Jun 30, 2016

Added jsx-self babel transform plugin #3540

Added jsx-self babel transform plugin #3540

Conversation

jimfb commented Jun 20, 2016

jimfb commented Jun 20, 2016

jimfb commented Jun 20, 2016

hzoo commented Jun 20, 2016 • edited Loading

codecov-io commented Jun 20, 2016 • edited Loading

Current coverage is 88.04%

hzoo commented Jun 20, 2016

jimfb commented Jun 21, 2016

hzoo commented Jun 21, 2016 • edited Loading

jimfb commented Jun 21, 2016

hzoo Jun 22, 2016

Choose a reason for hiding this comment

sebmck Jun 22, 2016

Choose a reason for hiding this comment

hzoo commented Jun 22, 2016

hzoo Jun 22, 2016

Choose a reason for hiding this comment

hzoo commented Jun 22, 2016

sophiebits commented Jun 27, 2016

sebmarkbage commented Jun 27, 2016

hzoo commented Jun 27, 2016 • edited Loading

jmm commented Jun 27, 2016

jimfb commented Jun 27, 2016

sophiebits commented Jun 27, 2016

jmm commented Jun 28, 2016

sophiebits commented Jun 28, 2016

jmm commented Jun 30, 2016

hzoo commented Jun 20, 2016 •

edited

Loading

codecov-io commented Jun 20, 2016 •

edited

Loading

hzoo commented Jun 21, 2016 •

edited

Loading

hzoo commented Jun 27, 2016 •

edited

Loading