Fixes Platform.OS and process.env.NODE_ENV inlining resulting in an invalid AST

Summary:
Resolves https://github.com/facebook/metro-bundler/issues/27

**Summary**

There is an edge case where tools like react-primitives need to assign to `Platform.OS`,
but the inline transformation results in an invalid AST.

When Platform.OS is unconditionally inlined, the following scenario:
````
Platform.OS = 'ios';
````
Is transformed to:
```
"ios"="ios"
````

And results in error `Property left of AssignmentExpression expected node to be of a type ["LVal"] but instead got "StringLiteral"**`.

See issue in react-primitives: https://github.com/lelandrichardson/react-primitives/issues/79

This patch checks whether the current node is on the left hand side of an AssignmentExpression
and skips the inlining when this is the case.

It also does the same for `process.env.NODE_ENV`, which suffers from the same problem. See related closed issue https://github.com/facebook/react-native/issues/7607#issuecomment-221425153 which was resolved by using bracket notation `process.env["NODE_ENV"]` instead.

**Test plan**

Unit tests included.

**Notes**

This is my first contribution to Metro, and haven't worked with Babel AST a lot, so constructive feedback on the implementation is welcome.
Closes https://github.com/facebook/metro-bundler/pull/45

Reviewed By: cpojer

Differential Revision: D5974615

Pulled By: mjesun

fbshipit-source-id: 224e63cef24f450b33e17ff9c0e0c0cea46bc70e
This commit is contained in:
Jani Eväkallio 2017-10-09 08:16:33 -07:00 committed by Facebook Github Bot
parent 475a580569
commit b2002609c1
2 changed files with 117 additions and 9 deletions

View File

@ -292,6 +292,90 @@ describe('inline constants', () => {
);
});
it(`doesn't replace Platform.OS in the code if Platform is the left hand side of an assignment expression`, () => {
const code = `function a() {
Platform.OS = "test"
}`;
const {ast} = inline('arbitrary.js', {code}, {platform: 'ios'});
expect(toString(ast)).toEqual(normalize(code));
});
it('replaces Platform.OS in the code if Platform is the right hand side of an assignment expression', () => {
const code = `function a() {
var a;
a = Platform.OS;
}`;
const {ast} = inline('arbitrary.js', {code}, {platform: 'ios'});
expect(toString(ast)).toEqual(
normalize(code.replace(/Platform\.OS/, '"ios"')),
);
});
it(`doesn't replace React.Platform.OS in the code if Platform is the left hand side of an assignment expression`, () => {
const code = `function a() {
React.Platform.OS = "test"
}`;
const {ast} = inline('arbitrary.js', {code}, {platform: 'ios'});
expect(toString(ast)).toEqual(normalize(code));
});
it('replaces React.Platform.OS in the code if Platform is the right hand side of an assignment expression', () => {
const code = `function a() {
var a;
a = React.Platform.OS;
}`;
const {ast} = inline('arbitrary.js', {code}, {platform: 'ios'});
expect(toString(ast)).toEqual(
normalize(code.replace(/React\.Platform\.OS/, '"ios"')),
);
});
it(`doesn't replace ReactNative.Platform.OS in the code if Platform is the left hand side of an assignment expression`, () => {
const code = `function a() {
ReactNative.Platform.OS = "test"
}`;
const {ast} = inline('arbitrary.js', {code}, {platform: 'ios'});
expect(toString(ast)).toEqual(normalize(code));
});
it('replaces ReactNative.Platform.OS in the code if Platform is the right hand side of an assignment expression', () => {
const code = `function a() {
var a;
a = ReactNative.Platform.OS;
}`;
const {ast} = inline('arbitrary.js', {code}, {platform: 'ios'});
expect(toString(ast)).toEqual(
normalize(code.replace(/ReactNative\.Platform\.OS/, '"ios"')),
);
});
it(`doesn't replace require("React").Platform.OS in the code if Platform is the left hand side of an assignment expression`, () => {
const code = `function a() {
require("React").Platform.OS = "test"
}`;
const {ast} = inline('arbitrary.js', {code}, {platform: 'ios'});
expect(toString(ast)).toEqual(normalize(code));
});
it('replaces require("React").Platform.OS in the code if Platform is the right hand side of an assignment expression', () => {
const code = `function a() {
var a;
a = require("React").Platform.OS;
}`;
const {ast} = inline('arbitrary.js', {code}, {platform: 'ios'});
expect(toString(ast)).toEqual(
normalize(code.replace(/require\("React"\)\.Platform\.OS/, '"ios"')),
);
});
it('replaces non-existing properties with `undefined`', () => {
const code = 'var a = Platform.select({ios: 1, android: 2})';
const {ast} = inline('arbitrary.js', {code}, {platform: 'doesnotexist'});
@ -313,6 +397,25 @@ describe('inline constants', () => {
);
});
it(`doesn't replace process.env.NODE_ENV in the code if NODE_ENV is the right hand side of an assignment expression`, () => {
const code = `function a() {
process.env.NODE_ENV = 'production';
}`;
const {ast} = inline('arbitrary.js', {code}, {dev: false});
expect(toString(ast)).toEqual(normalize(code));
});
it('replaces process.env.NODE_ENV in the code if NODE_ENV is the right hand side of an assignment expression', () => {
const code = `function a() {
var env;
env = process.env.NODE_ENV;
}`;
const {ast} = inline('arbitrary.js', {code}, {dev: false});
expect(toString(ast)).toEqual(
normalize(code.replace(/process\.env\.NODE_ENV/, '"production"')),
);
});
it('accepts an AST as input', function() {
const code = 'function ifDev(a,b){return __DEV__?a:b;}';
const {ast} = inline('arbitrary.hs', {ast: toAst(code)}, {dev: false});

View File

@ -65,6 +65,9 @@ function isImportOrGlobal(node, scope, patterns, isWrappedModule) {
);
}
const isLeftHandSideOfAssignmentExpression = (node, parent) =>
t.isAssignmentExpression(parent) && parent.left === node;
const isPlatformOS = (node, scope, isWrappedModule) =>
t.isIdentifier(node.property, os) &&
isImportOrGlobal(node.object, scope, [platform], isWrappedModule);
@ -127,15 +130,17 @@ const inlinePlugin = {
const scope = path.scope;
const opts = state.opts;
if (
isPlatformOS(node, scope, opts.isWrapped) ||
isReactPlatformOS(node, scope, opts.isWrapped)
) {
path.replaceWith(t.stringLiteral(opts.platform));
} else if (isProcessEnvNodeEnv(node, scope)) {
path.replaceWith(
t.stringLiteral(opts.dev ? 'development' : 'production'),
);
if (!isLeftHandSideOfAssignmentExpression(node, path.parent)) {
if (
isPlatformOS(node, scope, opts.isWrapped) ||
isReactPlatformOS(node, scope, opts.isWrapped)
) {
path.replaceWith(t.stringLiteral(opts.platform));
} else if (isProcessEnvNodeEnv(node, scope)) {
path.replaceWith(
t.stringLiteral(opts.dev ? 'development' : 'production'),
);
}
}
},
CallExpression(path, state) {