Fix inlinePlugin replacing __DEV__ in invalid Babel AST locations (…

…#1195)

Summary:
We've encountered this as part of a report on the `expo` repo: expo/expo#25965

There, an issue is caused by a dependency using an export alias named `__DEV__`, which the `inlinePlugin` is trying to replace:

```js
export { DEV as __DEV__ };
```

This only occurs in certain cases, since usually, as part of the iOS/Android builds, we would be applying the `inlinePlugin` only after export statements, where this issue may occur, have been transpiled away.

However, looking at this issue, I found more cases that would fail with the current implementation of the `inlinePlugin`. This is part of what I added in tests but not an exhaustive list:
- Shorthand object methods (`{ __DEV__() {} }`)
- Labelled statements (`__DEV__: {};`)
- Class methods (`class { __DEV__() {} }`)
- Optional property member access (`x?.__DEV__`)

All of these issues can be addressed by letting this replacement use `ReferencedIdentifier` as a visitor, rather than just `Identifier`.

Pull Request resolved: #1195

Test Plan: - Tests have been added to `inline-plugin-test.js` to reflect the mentioned cases.

Reviewed By: huntie

Differential Revision: D52909519

Pulled By: motiz88

fbshipit-source-id: 37a6459bc917701fe8474c33ccae41cb484e92f0

packages/metro-transform-plugins/src/__tests__/inline-plugin-test.js

@@ -16,45 +16,106 @@ const inlinePlugin = require('../inline-plugin');
 const stripFlow = require('@babel/plugin-transform-flow-strip-types');
 
 describe('inline constants', () => {
-  it('replaces __DEV__ in the code', () => {
-    const code = `
-      function a() {
-        var a = __DEV__ ? 1 : 2;
-        var b = a.__DEV__;
-        var c = function __DEV__(__DEV__) {};
-      }
-    `;
+  describe('__DEV__', () => {
+    it('replaces __DEV__ in the code', () => {
+      const code = `
+        function a() {
+          var a = __DEV__ ? 1 : 2;
+          var b = a.__DEV__;
+          var c = function __DEV__(__DEV__) {};
+        }
+      `;
 
-    compare([inlinePlugin], code, code.replace(/__DEV__/, 'false'), {
-      dev: false,
+      compare([inlinePlugin], code, code.replace(/__DEV__/, 'false'), {
+        dev: false,
+      });
     });
-  });
 
-  it("doesn't replace a local __DEV__ variable", () => {
-    const code = `
-      function a() {
-        var __DEV__ = false;
-        var a = __DEV__ ? 1 : 2;
-      }
-    `;
+    it("doesn't replace a local __DEV__ variable", () => {
+      const code = `
+        function a() {
+          var __DEV__ = false;
+          var a = __DEV__ ? 1 : 2;
+        }
+      `;
 
-    compare([inlinePlugin], code, code, {dev: false});
-  });
+      compare([inlinePlugin], code, code, {dev: false});
+    });
 
-  it("doesn't replace __DEV__ in an object property key", () => {
-    const code = `
-      const x = {
-        __DEV__: __DEV__
-      };
-    `;
+    it("doesn't replace __DEV__ in an object property key", () => {
+      const code = `
+        const x = { __DEV__: __DEV__ };
+      `;
 
-    const expected = `
-      const x = {
-        __DEV__: false
-      };
-    `;
+      const expected = `
+        const x = { __DEV__: false };
+      `;
+
+      compare([inlinePlugin], code, expected, {dev: false});
+    });
+
+    it("doesn't replace __DEV__ in an object shorthand method name", () => {
+      const code = `
+        const x = {
+          __DEV__() { return __DEV__; },
+        };
+      `;
+
+      const expected = `
+        const x = {
+          __DEV__() { return false; },
+        };
+      `;
+
+      compare([inlinePlugin], code, expected, {dev: false});
+    });
+
+    it("doesn't replace __DEV__ as a label of a block statement", () => {
+      const code = `
+        __DEV__: {
+          break __DEV__;
+        };
+      `;
+
+      compare([inlinePlugin], code, code, {dev: false});
+    });
+
+    it("doesn't replace __DEV__ as the name of a function declaration or call", () => {
+      const code = `
+        function __DEV__() { return false; }
+        const isDev = __DEV__();
+      `;
+
+      compare([inlinePlugin], code, code, {dev: false});
+    });
+
+    it("doesn't replace __DEV__ as the name of a class method", () => {
+      const code = `
+        class Test {
+          __DEV__() {}
+        }
+      `;
+
+      compare([inlinePlugin], code, code, {dev: false});
+    });
 
-    compare([inlinePlugin], code, expected, {dev: false});
+    it("doesn't replace __DEV__ as the name of an export alias", () => {
+      const code = `
+        const dev = true;
+        export { dev as __DEV__ };
+      `;
+
+      compare([inlinePlugin], code, code, {dev: false});
+    });
+
+    it("doesn't replace __DEV__ as the name of an optional property access", () => {
+      const code = `
+        x?.__DEV__;
+        x?.__DEV__();
+      `;
+
+      compare([inlinePlugin], code, code, {dev: false});
+    });
   });
 
   it('replaces Platform.OS in the code if Platform is a global', () => {

packages/metro-transform-plugins/src/inline-plugin.js

@@ -15,7 +15,6 @@ import type {PluginObj} from '@babel/core';
 import type {Binding, NodePath, Scope} from '@babel/traverse';
 import type {
   CallExpression,
-  Identifier,
   MemberExpression,
   Node,
   ObjectExpression,
@@ -84,12 +83,9 @@ function inlinePlugin(
     isIdentifier(node.object.object, processId) &&
     isGlobal(scope.getBinding(processId.name));
 
-  const isDev = (node: Identifier, parent: Node, scope: Scope): boolean =>
+  const isDev = (node: Node, parent: Node, scope: Scope): boolean =>
     isIdentifier(node, dev) &&
-    isGlobalOrFlowDeclared(scope.getBinding(dev.name)) &&
-    !isMemberExpression(parent) &&
-    // not { __DEV__: 'value'}
-    (!isObjectProperty(parent) || parent.value === node);
+    isGlobalOrFlowDeclared(scope.getBinding(dev.name));
 
   function findProperty(
     objectExpression: ObjectExpression,
@@ -134,7 +130,7 @@ function inlinePlugin(
 
   return {
     visitor: {
-      Identifier(path: NodePath<Identifier>, state: State): void {
+      ReferencedIdentifier(path: NodePath<Node>, state: State): void {
         if (!state.opts.dev && isDev(path.node, path.parent, path.scope)) {
           path.replaceWith(t.booleanLiteral(state.opts.dev));
         }