Removes un-needed React JSX key handling

Summary:
After adding some tests, it turns out that logic to apply keys to arrays was un-needed. This was because, after running the same code through React, the same errors occur when keys are omitted from arrays. We don't need to do any clever tricks to add keys, as it might actually break logic on things that don't need them. I've added the relevant tests and removed the key adding logic, which should help us with the de-duping work (https://github.com/facebook/prepack/issues/1150).
Closes https://github.com/facebook/prepack/pull/1151

Differential Revision: D6323878

Pulled By: trueadm

fbshipit-source-id: ec3fa90c48332ef0715e94335c558570040ed81c

scripts/test-react.js

@@ -133,6 +133,14 @@ describe("Test React", () => {
       await runTest(directory, "key-nesting.js");
     });
 
+    it("Key nesting 2", async () => {
+      await runTest(directory, "key-nesting-2.js");
+    });
+
+    it("Key nesting 3", async () => {
+      await runTest(directory, "key-nesting-3.js");
+    });
+
     it("Key change", async () => {
       await runTest(directory, "key-change.js");
     });

src/react/jsx.js

@@ -12,15 +12,13 @@
 import * as t from "babel-types";
 import type {
   BabelNodeExpression,
-  BabelNodeArrayExpression,
-  BabelNodeJSXElement,
   BabelNodeJSXMemberExpression,
   BabelNodeJSXIdentifier,
   BabelNodeIdentifier,
   BabelNodeMemberExpression,
 } from "babel-types";
 import invariant from "../invariant.js";
-import { isReactComponent, getUniqueReactElementKey } from "./utils";
+import { isReactComponent } from "./utils";
 
 export function convertExpressionToJSXIdentifier(
   expr: BabelNodeExpression,
@@ -78,56 +76,3 @@ export function convertJSXExpressionToIdentifier(
 export function convertKeyValueToJSXAttribute(key: string, expr: BabelNodeExpression) {
   return t.jSXAttribute(t.jSXIdentifier(key), expr.type === "StringLiteral" ? expr : t.jSXExpressionContainer(expr));
 }
-
-function addKeyToElement(astElement: BabelNodeJSXElement, key) {
-  let astAttributes = astElement.openingElement.attributes;
-  let existingKey = null;
-
-  for (let i = 0; i < astAttributes.length; i++) {
-    let astAttribute = astAttributes[i];
-
-    if (t.isJSXAttribute(astAttribute) && t.isJSXIdentifier(astAttribute.name) && astAttribute.name.name === "key") {
-      existingKey = astAttribute.value;
-      break;
-    }
-  }
-  if (existingKey === null) {
-    astAttributes.push(t.jSXAttribute(t.jSXIdentifier("key"), t.stringLiteral(key)));
-  }
-}
-
-export function applyKeysToNestedArray(
-  expr: BabelNodeArrayExpression,
-  isBase: boolean,
-  usedReactElementKeys: Set<string>
-): void {
-  let astElements = expr.elements;
-
-  if (Array.isArray(astElements)) {
-    for (let i = 0; i < astElements.length; i++) {
-      let astElement = astElements[i];
-
-      if (astElement != null) {
-        if (t.isJSXElement(astElement) && isBase === false) {
-          addKeyToElement((astElement: any), getUniqueReactElementKey("" + i, usedReactElementKeys));
-        } else if (t.isArrayExpression(astElement)) {
-          applyKeysToNestedArray((astElement: any), false, usedReactElementKeys);
-        } else if (astElement.type === "ConditionalExpression") {
-          let alternate = (astElement.alternate: any);
-          // it's common for conditions to be in an array, which means we need to check them for keys too
-          if (t.isJSXElement(alternate.type) && isBase === false) {
-            addKeyToElement(alternate, getUniqueReactElementKey("0" + i, usedReactElementKeys));
-          } else if (t.isArrayExpression(alternate.type)) {
-            applyKeysToNestedArray(alternate, false, usedReactElementKeys);
-          }
-          let consequent = (astElement.consequent: any);
-          if (t.isJSXElement(consequent.type) && isBase === false) {
-            addKeyToElement(consequent, getUniqueReactElementKey("1" + i, usedReactElementKeys));
-          } else if (t.isArrayExpression(consequent.type)) {
-            applyKeysToNestedArray(consequent, false, usedReactElementKeys);
-          }
-        }
-      }
-    }
-  }
-}

src/serializer/ResidualHeapSerializer.js

@@ -29,15 +29,10 @@ import {
   NativeFunctionValue,
   UndefinedValue,
 } from "../values/index.js";
-import {
-  convertExpressionToJSXIdentifier,
-  convertKeyValueToJSXAttribute,
-  applyKeysToNestedArray,
-} from "../react/jsx.js";
+import { convertExpressionToJSXIdentifier, convertKeyValueToJSXAttribute } from "../react/jsx.js";
 import { isReactElement } from "../react/utils.js";
 import * as t from "babel-types";
 import type {
-  BabelNodeArrayExpression,
   BabelNodeExpression,
   BabelNodeStatement,
   BabelNodeIdentifier,
@@ -744,16 +739,14 @@ export class ResidualHeapSerializer {
       // we do this to ensure child JSXElements can get keys assigned if needed
       this.serializedValues.add(child);
       let reactChild = this._serializeValueObject(((child: any): ObjectValue));
-      if (reactChild.leadingComments !== null) {
+      if (reactChild.leadingComments != null) {
         return t.jSXExpressionContainer(reactChild);
       }
       return reactChild;
     }
     const expr = this.serializeValue(child);
 
-    if (t.isArrayExpression(expr)) {
-      applyKeysToNestedArray(((expr: any): BabelNodeArrayExpression), true, this.react.usedReactElementKeys);
-    } else if (t.isStringLiteral(expr) || t.isNumericLiteral(expr)) {
+    if (t.isStringLiteral(expr) || t.isNumericLiteral(expr)) {
       return t.jSXText(((expr: any).value: string) + "");
     } else if (t.isJSXElement(expr)) {
       return expr;

test/react/functional-components/key-nesting-2.js

@@ -0,0 +1,31 @@
+var React = require('react');
+
+// the JSX transform converts to React, so we need to add it back in
+this['React'] = React;
+
+function Child() {
+  return [
+    <span key="a" />,
+    <span key="b" />
+  ]
+}
+
+function App() {
+  return <div>
+    <span />
+    <Child />
+    <Child />
+		<span />
+  </div>
+}
+
+App.getTrials = function(renderer, Root) {
+  renderer.update(<Root />);
+  return [['key nesting 2', renderer.toJSON()]];
+};
+
+if (this.__registerReactComponentRoot) {
+  __registerReactComponentRoot(App);
+}
+
+module.exports = App;

test/react/functional-components/key-nesting-3.js

@@ -0,0 +1,33 @@
+var React = require('react');
+
+// the JSX transform converts to React, so we need to add it back in
+this['React'] = React;
+
+function Child() {
+	var x = [];
+
+	for (let i = 0; i < 10; i++) {
+		x.push(<span key={i} />)
+	}
+  return x;
+}
+
+function App() {
+  return <div>
+    <span />
+    <Child />
+    <Child />
+		<span />
+  </div>
+}
+
+App.getTrials = function(renderer, Root) {
+  renderer.update(<Root />);
+  return [['key nesting 3', renderer.toJSON()]];
+};
+
+if (this.__registerReactComponentRoot) {
+  __registerReactComponentRoot(App);
+}
+
+module.exports = App;