Skip to content

Commit

Permalink
Fix BABEL_ENV resolution in babel transformer (#446)
Browse files Browse the repository at this point in the history
Summary:
**Summary**

Fix a regression introduced in `v0.53.0` via c0f6693

We're using some babel plugins in our React Native applications and noticed some weird issues after upgrading to RN 0.60. Upon further investigation, it appeared the issue is coming from metro babel transformer setting `BABEL_ENV` to an incorrect value.

The problem is due to the very unintuitive behavior of assigning values on `process.env` as described in [nodejs doc](https://nodejs.org/dist/latest-v10.x/docs/api/process.html) :
> Assigning a property on process.env will implicitly convert the value to a string. This behavior is deprecated. Future versions of Node.js may throw an error when the value is not a string, number, or boolean.

For example, from a node REPL :
```
> process.env.BABEL_ENV = undefined
undefined
> process.env.BABEL_ENV === undefined
false
> process.env.BABEL_ENV === 'undefined'
true
```

With `BABEL_ENV` not set (undefined), and `options.dev=false` the first run of the `transform` function is properly setting `BABEL_ENV` to `production`. Then, prior to exiting the function, it is [restoring `BABEL_ENV` to its initial value](https://github.com/facebook/metro/blob/v0.56.0/packages/metro-babel-transformer/src/index.js#L74) that was captured upon entering the function. In this case `undefined`, so `BABEL_ENV` is now set to `'undefined'` (string). On subsequent runs of the function it will therefore set `BABEL_ENV` to `'undefined'`.

Given this, the fix is actually straightforward. Only assign `OLD_BABEL_ENV` to `BABEL_ENV` if `OLD_BABEL_ENV` is defined, to avoid running into the problem of having `BABEL_ENV` set to `'undefined'`.
Pull Request resolved: #446

Differential Revision: D17317325

Pulled By: cpojer

fbshipit-source-id: 1b00c126279124470a2de907692363e5aec96ae3
  • Loading branch information
belemaire authored and facebook-github-bot committed Sep 11, 2019
1 parent ca68db1 commit c509a89
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
4 changes: 3 additions & 1 deletion packages/metro-babel-transformer/src/index.js
Expand Up @@ -71,7 +71,9 @@ function transform({filename, options, plugins, src}: BabelTransformerArgs) {

return {ast, functionMap};
} finally {
process.env.BABEL_ENV = OLD_BABEL_ENV;
if (OLD_BABEL_ENV) {
process.env.BABEL_ENV = OLD_BABEL_ENV;
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion packages/metro-react-native-babel-transformer/src/index.js
Expand Up @@ -172,7 +172,9 @@ function transform({filename, options, src, plugins}: BabelTransformerArgs) {

return {ast: result.ast, functionMap};
} finally {
process.env.BABEL_ENV = OLD_BABEL_ENV;
if (OLD_BABEL_ENV) {
process.env.BABEL_ENV = OLD_BABEL_ENV;
}
}
}

Expand Down

0 comments on commit c509a89

Please sign in to comment.