Skip to content

Conversation

@frantic
Copy link
Contributor

@frantic frantic commented Mar 7, 2016

Putting this up as request for comments.

The PR adds transform-react-jsx-source to the list of plugins that come by default with the react-native preset. It will enable the use of a bunch of really cool tooling around JSX, however those are generally useful only in development mode. Is changing react-native preset the right thing to do in this case? Is there a way to enable this transform only in DEV? Should I add this somewhere else?

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @skevy to be a potential reviewer.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Mar 7, 2016
@ide
Copy link
Contributor

ide commented Mar 8, 2016

babel-preset-react-native is likely the right place -- we consolidate all babel plugins in this preset so that people can use the react-native preset in custom .babelrc files and automatically get the same configuration as a plain RN project.

Making the plugin run only in dev is trickier though.... perhaps we need to create 2 presets: babel-preset-react-native andbabel-preset-react-native/dev. The packager would choose which to load based on the "dev" param in the request for the JS bundle. Where this gets tricky is with custom .babelrc files since AFAIK you can't have a separate .babelrc for dev mode.

Another option is to have the packager explicitly load this plugin (ex:

const extraPlugins = [externalHelpersPlugin];
), and we'd not add it to rn-babelrc.json.

cc @davidaurelio

@janicduplessis
Copy link
Contributor

There is a env option for the babelrc file, maybe it could work here. https://babeljs.io/docs/usage/babelrc/#env-option

Otherwise we could set an environment variable (or something else if there is a better way to pass data) in the packager before running the transforms and check it in the preset.

@ide
Copy link
Contributor

ide commented Mar 8, 2016

Fantastic -- let's set BABEL_ENV = process.env.BABEL_ENV || (__DEV__ ? 'development' : 'production') then?

@skevy skevy self-assigned this Mar 8, 2016
@frantic
Copy link
Contributor Author

frantic commented Mar 10, 2016

Lmk how you want to proceed, I don't have strong opinions regarding babel configuration, but I can't wait to show you a demo of what this thing enables us to do!

@iamdustan
Copy link

I want to see this demo!

@janicduplessis
Copy link
Contributor

You can try something like this:

{
  plugins: resolvePlugins([
    ...
  ]),
  env: {
    development: {
      plugins: resolvePlugins(['babel-plugin-transform-react-jsx-source']),
    },
  },
}

I'm not sure the env config only works in .babelrc files. Then we should probably set the BABEL_ENV environment variable in the packager before running the transform based on the value of the dev param of the packager for the generated bundle.

@janicduplessis
Copy link
Contributor

Maybe @kittens can chip in here see if we are doing this right :)

@ide
Copy link
Contributor

ide commented Mar 10, 2016

@frantic could you try using Babel's env option that @janicduplessis linked to, and see if you can make it so that the plugin runs only in dev mode?

@satya164
Copy link
Contributor

Curious, what if someone is relying on a __source prop? Highly unlikely, but still!

@sebmck
Copy link
Contributor

sebmck commented Mar 11, 2016

Yeah using the env option and setting the BABEL_ENV is the right way to conditionally load plugins.

@ghost
Copy link

ghost commented Apr 15, 2016

@frantic do you have any updates for this pull request? It's been a while so we wanted to check in and see if you've looked at the requested changes.

@facebook-github-bot
Copy link
Contributor

@frantic updated the pull request.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@frantic updated the pull request.

@frantic
Copy link
Contributor Author

frantic commented Apr 15, 2016

Demo of how this will be useful (the commit that adds Inspector button is landing soon):

zcichu7kem

@ghost
Copy link

ghost commented Apr 16, 2016

@martinbigio would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

@frantic
Copy link
Contributor Author

frantic commented May 5, 2016

@kittens does the code with BABEL_ENV look good? If so, I'm going to double-check the tests and land this.

@frantic frantic force-pushed the jsx-source-preset branch from dca61fe to b725ca1 Compare May 5, 2016 22:17
@ghost
Copy link

ghost commented May 5, 2016

@frantic updated the pull request.

@ghost
Copy link

ghost commented May 6, 2016

@skevy would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

package.json Outdated
"babel-plugin-external-helpers": "^6.5.0",
"babel-polyfill": "^6.6.1",
"babel-preset-react-native": "^1.7.0",
"babel-preset-react-native": "frantic/babel-preset-react-native",
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this, please. We will have to update this seperately (i.e. after publishing to npm)

ghost pushed a commit that referenced this pull request May 18, 2016
Summary:
Depends on #6351

Now you can open file directly from Elements Inspector!

Credit for the original implementation goes to jaredly

![zcichu7kem](https://cloud.githubusercontent.com/assets/192222/14573876/cb100f6e-030c-11e6-925f-6a6dff510145.gif)

**Test plan**

Made sure it doesn't crash the app with or without #6351 (i.e. can be merged safely before #6351 gets in).
Closes #7005

Differential Revision: D3313714

Pulled By: frantic

fbshipit-source-id: 3b80abd3e81a0db5ca5136e2d2c94c775fa04f3a
@ghost ghost closed this in 858643d May 18, 2016
poppybank pushed a commit to poppybank/react-native that referenced this pull request May 19, 2016
Summary:
Putting this up as request for comments.

The PR adds [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) to the list of plugins that come by default with the `react-native` preset. It will enable the use of a bunch of really cool tooling around JSX, however those are generally useful only in development mode. Is changing `react-native` preset the right thing to do in this case? Is there a way to enable this transform only in DEV? Should I add this somewhere else?
Closes facebook#6351

Differential Revision: D3302906

Pulled By: frantic

fbshipit-source-id: 012d3a4142168f9f90d30d1686115d4dc3996eb9
zebulgar pushed a commit to nightingale/react-native that referenced this pull request Jun 18, 2016
Summary:
Depends on facebook#6351

Now you can open file directly from Elements Inspector!

Credit for the original implementation goes to jaredly

![zcichu7kem](https://cloud.githubusercontent.com/assets/192222/14573876/cb100f6e-030c-11e6-925f-6a6dff510145.gif)

**Test plan**

Made sure it doesn't crash the app with or without facebook#6351 (i.e. can be merged safely before facebook#6351 gets in).
Closes facebook#7005

Differential Revision: D3313714

Pulled By: frantic

fbshipit-source-id: 3b80abd3e81a0db5ca5136e2d2c94c775fa04f3a
zebulgar pushed a commit to nightingale/react-native that referenced this pull request Jun 18, 2016
Summary:
Putting this up as request for comments.

The PR adds [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) to the list of plugins that come by default with the `react-native` preset. It will enable the use of a bunch of really cool tooling around JSX, however those are generally useful only in development mode. Is changing `react-native` preset the right thing to do in this case? Is there a way to enable this transform only in DEV? Should I add this somewhere else?
Closes facebook#6351

Differential Revision: D3302906

Pulled By: frantic

fbshipit-source-id: 012d3a4142168f9f90d30d1686115d4dc3996eb9
@oliviertassinari
Copy link

Notice that highjacking the BABEL_ENV forced me to change the name of my NODE_ENV to development.
I'm using a different environment name to make a React Dom and React Native works under the same directory.
For more details, you can have a look at react-swipeable-views/pull#111.

@satya164
Copy link
Contributor

satya164 commented Jun 25, 2016

@oliviertassinari What do you mean by to make a React Dom and React Native works under the same directory?

@oliviertassinari
Copy link

oliviertassinari commented Jun 26, 2016

@satya164 Well, I have two implementations of the same components for two platforms inside the same folder. Basically, x.js is targeting the react-dom, and x.native.js is targeting react-native.

To make this work, I have a single .babelrc at the root of the project. And I'm using 4 differents babel environment in my .babelrc.

  • one for the doc site in development mode
  • one for the doc site in production mode
  • one for the doc native app in development mode
  • one for the npm release

jaredly added a commit to jaredly/react-devtools that referenced this pull request Aug 5, 2016
To take advantage of this, use the [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) babel plugin.

The `_source` property was added to react [last year](facebook/react@009b7c8) and can be used by development tools to track the source file & line of react elements. React Native is also [taking advantage of it](facebook/react-native#6351).

Test Plan:
- install the [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) babel plugin
- open the react devtools to an element that was created in your app (can't
  have been created by a library you're using, because those are probably
  pre-transpiled & ignored by babel)
jaredly added a commit to jaredly/react-devtools that referenced this pull request Aug 5, 2016
To take advantage of this, use the [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) babel plugin.

The `_source` property was added to react [last year](facebook/react@009b7c8) and can be used by development tools to track the source file & line of react elements. React Native is also [taking advantage of it](facebook/react-native#6351).

Test Plan:
- install the [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) babel plugin
- open the react devtools to an element that was created in your app (can't
  have been created by a library you're using, because those are probably
  pre-transpiled & ignored by babel)
jaredly added a commit to facebook/react-devtools that referenced this pull request Aug 17, 2016
* Handle & render the _source property

To take advantage of this, use the [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) babel plugin.

The `_source` property was added to react [last year](facebook/react@009b7c8) and can be used by development tools to track the source file & line of react elements. React Native is also [taking advantage of it](facebook/react-native#6351).

Test Plan:
- install the [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) babel plugin
- open the react devtools to an element that was created in your app (can't
  have been created by a library you're using, because those are probably
  pre-transpiled & ignored by babel)

* add readme line

* fix type check

* remove `shortFilePath`
bubblesunyum pushed a commit to iodine/react-native that referenced this pull request Aug 23, 2016
Summary:
Depends on facebook#6351

Now you can open file directly from Elements Inspector!

Credit for the original implementation goes to jaredly

![zcichu7kem](https://cloud.githubusercontent.com/assets/192222/14573876/cb100f6e-030c-11e6-925f-6a6dff510145.gif)

**Test plan**

Made sure it doesn't crash the app with or without facebook#6351 (i.e. can be merged safely before facebook#6351 gets in).
Closes facebook#7005

Differential Revision: D3313714

Pulled By: frantic

fbshipit-source-id: 3b80abd3e81a0db5ca5136e2d2c94c775fa04f3a
bubblesunyum pushed a commit to iodine/react-native that referenced this pull request Aug 23, 2016
Summary:
Putting this up as request for comments.

The PR adds [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) to the list of plugins that come by default with the `react-native` preset. It will enable the use of a bunch of really cool tooling around JSX, however those are generally useful only in development mode. Is changing `react-native` preset the right thing to do in this case? Is there a way to enable this transform only in DEV? Should I add this somewhere else?
Closes facebook#6351

Differential Revision: D3302906

Pulled By: frantic

fbshipit-source-id: 012d3a4142168f9f90d30d1686115d4dc3996eb9
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
Summary:
Depends on facebook#6351

Now you can open file directly from Elements Inspector!

Credit for the original implementation goes to jaredly

![zcichu7kem](https://cloud.githubusercontent.com/assets/192222/14573876/cb100f6e-030c-11e6-925f-6a6dff510145.gif)

**Test plan**

Made sure it doesn't crash the app with or without facebook#6351 (i.e. can be merged safely before facebook#6351 gets in).
Closes facebook#7005

Differential Revision: D3313714

Pulled By: frantic

fbshipit-source-id: 3b80abd3e81a0db5ca5136e2d2c94c775fa04f3a
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
Summary:
Putting this up as request for comments.

The PR adds [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) to the list of plugins that come by default with the `react-native` preset. It will enable the use of a bunch of really cool tooling around JSX, however those are generally useful only in development mode. Is changing `react-native` preset the right thing to do in this case? Is there a way to enable this transform only in DEV? Should I add this somewhere else?
Closes facebook#6351

Differential Revision: D3302906

Pulled By: frantic

fbshipit-source-id: 012d3a4142168f9f90d30d1686115d4dc3996eb9
cpojer pushed a commit to facebook/metro that referenced this pull request Jan 26, 2017
Summary:
Putting this up as request for comments.

The PR adds [transform-react-jsx-source](https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-react-jsx-source) to the list of plugins that come by default with the `react-native` preset. It will enable the use of a bunch of really cool tooling around JSX, however those are generally useful only in development mode. Is changing `react-native` preset the right thing to do in this case? Is there a way to enable this transform only in DEV? Should I add this somewhere else?
Closes facebook/react-native#6351

Differential Revision: D3302906

Pulled By: frantic

fbshipit-source-id: 012d3a4142168f9f90d30d1686115d4dc3996eb9
@okonet
Copy link

okonet commented Jun 14, 2017

@davidaurelio
Copy link
Contributor

@okonet that’s for third-party code relying on it.

@davidaurelio
Copy link
Contributor

I would be happy to remove it, but I am not sure how many user setups that would affect

@okonet
Copy link

okonet commented Jun 15, 2017

Can we rewrite the check so it won't overwrite the passed env but only check if it's development?

@davidaurelio
Copy link
Contributor

Maybe we can not overwrite passed-in envs. Please be aware that neither env.BABEL_ENV nor env.NODE_ENV are used for caching purposes, i.e. it is easy to poison your local cache by using different values, or not having files re-transformed when this value changes.

@janicduplessis
Copy link
Contributor

The goal of this was to pass the packager dev env to babel so if you pass &dev=true it will run plugins in the env.development config.

@janicduplessis
Copy link
Contributor

If it causes issues there are probably other ways to do it.

@okonet
Copy link

okonet commented Jun 19, 2017

@janicduplessis yeah, the fact it overwrites and ignores BABEL_ENV is causing issues in multi-compile setups (web & RN for example)

See #8723 and #13733

I could do a PR with a potential fix but I don't have any experience with RN yet.

@okonet
Copy link

okonet commented Jun 19, 2017

facebook-github-bot pushed a commit to facebook/metro that referenced this pull request Feb 25, 2019
Summary:
**Summary**

Allows custom non-dev `BABEL_ENV`s besides `'production'`. With respect to facebook/react-native#6351 (comment).

**Test plan**

This change will break non-dev builds for people who for any reason has custom `BABEL_ENV` set but expects it to behave like `'production`' (current behavior). Though it can be easily fixed either by renaming their `env` from `production` to the value of custom `BABEL_ENV`, or by changing `BABEL_ENV` from custom to `'production'`.

Other than this unusual case, nothing should change.
Pull Request resolved: #364

Differential Revision: D14206886

Pulled By: cpojer

fbshipit-source-id: 3e4ef2b2beb4ce94335f72cc974d35312f847a9f
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.