Skip to content

Conversation

@nodkz
Copy link

@nodkz nodkz commented Jun 19, 2018

I have a complex .babelrc.js configuration (for server, for web and also for react-native (metro)).

It would be nice to allow for developers to provide its own BABEL_ENV name.
For now, it is forced to be production or development. And it's bad developer experience.

Of course METRO_BABEL_ENV is not a good solution. Better will be to use already defined BABEL_ENV variable from process, but if it not defined use production or development as a fallback.

With proposed changes it can be used in following manner:

$ METRO_BABEL_ENV=native_dev node node_modules/react-native/local-cli/cli.js start

or even better will be (but it may introduce some problems in existing apps)

$ BABEL_ENV=native_dev node node_modules/react-native/local-cli/cli.js start

Thoughts?

I have a complex .babelrc.js configuration (for server, for web and also for react-native (metro)).

It would be nice to allow for developers to provide its own `BABEL_ENV` name.
For now, it is forced to be `production` or `development`. And it's bad developer experience.

Of course `METRO_BABEL_ENV` is not a good solution. Better will be to use already defined BABEL_ENV variable from process, but if it not defined use `production` or `development` as a fallback.

With proposed changes it can be used in following manner:
```bash
$ METRO_BABEL_ENV=native_dev node node_modules/react-native/local-cli/cli.js start
```
or even better will be (but it may introduce some problems in existing apps)
```bash
$ BABEL_ENV=native_dev node node_modules/react-native/local-cli/cli.js start
```

Thoughts?
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 19, 2018
@lebedev
Copy link
Contributor

lebedev commented Aug 2, 2018

but it may introduce some problems in existing apps

How so? I'm not entirely sure, but I can only think of apps that export some "unusual" BABEL_ENV which is currently ignored. In most (if not all) cases apps are built is some CI-like environments, where environment variables are either not set, or set explicitly. Are there use cases where an app is built and some other babel environment is used at the same time (or same script, or same build process)?

Better will be to use already defined BABEL_ENV variable from process, but if it not defined use production or development as a fallback.

I'd vote for this.

My use case is that I need to be able to specify several different configs with transform-define plugin, that defines various constants like URLs, tokens etc. per environment. In order to be able to test release builds with staging URLs and tokens, my production Babel environment mostly duplicates development environment, and a script in CI is used for patching .babelrc with real constants before building true production builds. It would be much simpler if I could just define and use another babel environment for this.

@nodkz currently your PR doesn't pass some tests. Maybe you can fix (to pass run-js-checks tests) and rebase it (to probably pass test-node-6 tests)? Also if you yourself don't think that using another constant (METRO_BABEL_ENV) is a good solution, why not modify the PR according to the idea that you think is better?

@nodkz
Copy link
Author

nodkz commented Aug 10, 2018

@frantic maybe remember why was added replacement for standard BABEL_ENV?
That's why I add METRO_BABEL_ENV, of course, I also like BABEL_ENV.

function transform({filename, options, src, plugins}: Params) {
const OLD_BABEL_ENV = process.env.BABEL_ENV;
/* $FlowFixMe(>=0.68.0 site=react_native_fb) This comment suppresses an error
* found when Flow v0.68 was deployed. To see the error delete this comment
* and run Flow. */
process.env.BABEL_ENV = process.env.METRO_BABEL_ENV || (options.dev ? 'development' : 'production');
try {
const babelConfig = buildBabelConfig(filename, options, plugins);
const {ast} = transformSync(src, {
// ES modules require sourceType='module' but OSS may not always want that
sourceType: 'unambiguous',
...babelConfig,
ast: true,
});
return {ast};
} finally {
process.env.BABEL_ENV = OLD_BABEL_ENV;
}
}

@angly-cat I fixed lint errors. With failed node6 I don't know what to do.

@lebedev
Copy link
Contributor

lebedev commented Aug 10, 2018

maybe remember why was added replacement for standard BABEL_ENV?

Check the MR where this was added ans this comment in particular. If that's the idea, then to comply with it the code should be something like process.env.BABEL_ENV = options.dev ? 'development' : process.env.BABEL_ENV || 'production';?

@cpojer
Copy link
Contributor

cpojer commented Feb 25, 2019

Sorry for the super late response. It seems like now this is pretty outdated and files have moved.

If you want to send this PR again with the proposal by @angly-cat, I'm happy to merge it much faster :)

@cpojer cpojer closed this Feb 25, 2019
@lebedev lebedev mentioned this pull request Feb 25, 2019
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.

4 participants