Small tidy up of ReactElement logic

Summary:
Release notes: none

This PR cleans up the logic in how ReactElement properties are visited, moving them to the right place in the codebase. `react/utils/getProperty` also now properly deals with missing properties and throws an introspection error for getters/setters.
Closes https://github.com/facebook/prepack/pull/1507

Differential Revision: D7092708

Pulled By: trueadm

fbshipit-source-id: 4064a0cf48b633bd77012afb99e514ac5716e062

scripts/__snapshots__/test-react.js.snap

@@ -133,6 +133,13 @@ ReactStatistics {
 }
 `;
 
+exports[`Test React with JSX input, JSX output Functional component folding Delete element prop key 1`] = `
+ReactStatistics {
+  "inlinedComponents": 3,
+  "optimizedTrees": 1,
+}
+`;
+
 exports[`Test React with JSX input, JSX output Functional component folding Dynamic context 1`] = `
 ReactStatistics {
   "inlinedComponents": 1,
@@ -602,6 +609,13 @@ ReactStatistics {
 }
 `;
 
+exports[`Test React with JSX input, create-element output Functional component folding Delete element prop key 1`] = `
+ReactStatistics {
+  "inlinedComponents": 3,
+  "optimizedTrees": 1,
+}
+`;
+
 exports[`Test React with JSX input, create-element output Functional component folding Dynamic context 1`] = `
 ReactStatistics {
   "inlinedComponents": 1,
@@ -1071,6 +1085,13 @@ ReactStatistics {
 }
 `;
 
+exports[`Test React with create-element input, JSX output Functional component folding Delete element prop key 1`] = `
+ReactStatistics {
+  "inlinedComponents": 3,
+  "optimizedTrees": 1,
+}
+`;
+
 exports[`Test React with create-element input, JSX output Functional component folding Dynamic context 1`] = `
 ReactStatistics {
   "inlinedComponents": 1,
@@ -1540,6 +1561,13 @@ ReactStatistics {
 }
 `;
 
+exports[`Test React with create-element input, create-element output Functional component folding Delete element prop key 1`] = `
+ReactStatistics {
+  "inlinedComponents": 3,
+  "optimizedTrees": 1,
+}
+`;
+
 exports[`Test React with create-element input, create-element output Functional component folding Dynamic context 1`] = `
 ReactStatistics {
   "inlinedComponents": 1,

scripts/test-react.js

@@ -297,6 +297,10 @@ function runTestSuite(outputJsx, shouldTranspileSource) {
         await runTest(directory, "key-change.js");
       });
 
+      it("Delete element prop key", async () => {
+        await runTest(directory, "delete-element-prop-key.js");
+      });
+
       it("Key change with fragments", async () => {
         await runTest(directory, "key-change-fragments.js");
       });

src/react/utils.js

@@ -38,8 +38,17 @@ import AbstractValue from "../values/AbstractValue";
 export type ReactSymbolTypes = "react.element" | "react.fragment" | "react.portal" | "react.return" | "react.call";
 
 export function isReactElement(val: Value): boolean {
-  if (val instanceof ObjectValue && val.properties.has("$$typeof")) {
-    let realm = val.$Realm;
+  if (!(val instanceof ObjectValue)) {
+    return false;
+  }
+  let realm = val.$Realm;
+  if (!realm.react.enabled) {
+    return false;
+  }
+  if (realm.react.reactElements.has(val)) {
+    return true;
+  }
+  if (val.properties.has("$$typeof")) {
     let $$typeof = Get(realm, val, "$$typeof");
     let globalObject = realm.$GlobalObject;
     let globalSymbolValue = Get(realm, globalObject, "Symbol");
@@ -50,7 +59,12 @@ export function isReactElement(val: Value): boolean {
       }
     } else if ($$typeof instanceof SymbolValue) {
       let symbolFromRegistry = realm.globalSymbolRegistry.find(e => e.$Symbol === $$typeof);
-      return symbolFromRegistry !== undefined && symbolFromRegistry.$Key === "react.element";
+      let _isReactElement = symbolFromRegistry !== undefined && symbolFromRegistry.$Key === "react.element";
+      if (_isReactElement) {
+        // add to Set to speed up future lookups
+        realm.react.reactElements.add(val);
+        return true;
+      }
     }
   }
   return false;
@@ -382,7 +396,9 @@ export function getProperty(realm: Realm, object: ObjectValue, property: string
   } else {
     binding = object.symbols.get(property);
   }
-  invariant(binding);
+  if (!binding) {
+    return realm.intrinsics.undefined;
+  }
   let descriptor = binding.descriptor;
 
   if (!descriptor) {
@@ -391,7 +407,10 @@ export function getProperty(realm: Realm, object: ObjectValue, property: string
   let value;
   if (descriptor.value) {
     value = descriptor.value;
+  } else if (descriptor.get || descriptor.set) {
+    AbstractValue.reportIntrospectionError(object, `react/utils/getProperty unsupported getter/setter property`);
+    throw new FatalError();
   }
-  invariant(value instanceof Value, `ReactElementSet could not get value for`, object, property);
+  invariant(value instanceof Value, `react/utils/getProperty should not be called on internal properties`);
   return value;
 }

src/realm.js

@@ -188,6 +188,7 @@ export class Realm {
       output: opts.reactOutput || "create-element",
       hoistableFunctions: new WeakMap(),
       hoistableReactElements: new WeakMap(),
+      reactElements: new Set(),
       symbols: new Map(),
     };
 
@@ -255,6 +256,7 @@ export class Realm {
     hoistableFunctions: WeakMap<FunctionValue, boolean>,
     hoistableReactElements: WeakMap<ObjectValue, boolean>,
     output?: ReactOutputTypes,
+    reactElements: Set<ObjectValue>,
     symbols: Map<ReactSymbolTypes, SymbolValue>,
   };
   stripFlow: boolean;

src/serializer/ResidualHeapInspector.js

@@ -27,6 +27,7 @@ import {
 } from "../values/index.js";
 import invariant from "../invariant.js";
 import { Logger } from "../utils/logger.js";
+import { isReactElement } from "../react/utils.js";
 
 type TargetIntegrityCommand = "freeze" | "seal" | "preventExtensions" | "";
 
@@ -129,6 +130,15 @@ export class ResidualHeapInspector {
         // length property has the correct descriptor values
         return true;
       }
+    } else if (isReactElement(val)) {
+      // we don't want to visit/serialize $$typeof, _owner and _store properties
+      // as these are all internals of JSX/createElement
+      if (key === "$$typeof" || key === "_owner" || key === "_store") {
+        return true;
+      }
+      if ((key === "ref" || key === "key") && desc.value === this.realm.intrinsics.null) {
+        return true;
+      }
     } else if (val instanceof FunctionValue) {
       if (key === "length") {
         if (desc.value === undefined) {

src/serializer/ResidualHeapVisitor.js

@@ -174,7 +174,7 @@ export class ResidualHeapVisitor {
     let { skipPrototype, constructor } = getObjectPrototypeMetadata(this.realm, obj);
 
     // visit properties
-    if (kind !== "ReactElement") {
+    if (!isReactElement(obj)) {
       for (let [symbol, propertyBinding] of obj.symbols) {
         invariant(propertyBinding);
         let desc = propertyBinding.descriptor;
@@ -186,15 +186,6 @@ export class ResidualHeapVisitor {
 
     // visit properties
     for (let [propertyBindingKey, propertyBindingValue] of obj.properties) {
-      // we don't want to the $$typeof or _owner/_store properties
-      // as this is contained within the JSXElement, otherwise
-      // they we be need to be emitted during serialization
-      if (
-        kind === "ReactElement" &&
-        (propertyBindingKey === "$$typeof" || propertyBindingKey === "_owner" || propertyBindingKey === "_store")
-      ) {
-        continue;
-      }
       // we don't want to visit these as we handle the serialization ourselves
       // via a different logic route for classes
       let descriptor = propertyBindingValue.descriptor;
@@ -221,7 +212,7 @@ export class ResidualHeapVisitor {
     }
 
     // prototype
-    if (kind !== "ReactElement" && !skipPrototype) {
+    if (!isReactElement(obj) && !skipPrototype) {
       // we don't want to the ReactElement prototype visited
       // as this is contained within the JSXElement, otherwise
       // they we be need to be emitted during serialization

src/serializer/ResidualReactElementSerializer.js

@@ -53,30 +53,24 @@ export class ResidualReactElementSerializer {
     let propsValue = getProperty(this.realm, val, "props");
     let waitForProperties = [val, typeValue, keyValue, refValue, propsValue];
 
-    invariant(typeValue !== null, "ReactElement type of null");
-
     let attributes = [];
     let children = [];
 
-    if (keyValue !== null) {
+    if (keyValue !== this.realm.intrinsics.null) {
       let keyExpr = this.residualHeapSerializer.serializeValue(keyValue);
-      if (keyExpr.type !== "NullLiteral") {
-        if (this.reactOutput === "jsx") {
-          this._addSerializedValueToJSXAttriutes("key", keyExpr, attributes);
-        } else if (this.reactOutput === "create-element") {
-          this._addSerializedValueToObjectProperty("key", keyExpr, attributes);
-        }
+      if (this.reactOutput === "jsx") {
+        this._addSerializedValueToJSXAttriutes("key", keyExpr, attributes);
+      } else if (this.reactOutput === "create-element") {
+        this._addSerializedValueToObjectProperty("key", keyExpr, attributes);
       }
     }
 
-    if (refValue !== null) {
+    if (refValue !== this.realm.intrinsics.null) {
       let refExpr = this.residualHeapSerializer.serializeValue(refValue);
-      if (refExpr.type !== "NullLiteral") {
-        if (this.reactOutput === "jsx") {
-          this._addSerializedValueToJSXAttriutes("ref", refExpr, attributes);
-        } else if (this.reactOutput === "create-element") {
-          this._addSerializedValueToObjectProperty("ref", refExpr, attributes);
-        }
+      if (this.reactOutput === "jsx") {
+        this._addSerializedValueToJSXAttriutes("ref", refExpr, attributes);
+      } else if (this.reactOutput === "create-element") {
+        this._addSerializedValueToObjectProperty("ref", refExpr, attributes);
       }
     }
 

test/react/functional-components/delete-element-prop-key.js

@@ -0,0 +1,40 @@
+var React = require('react');
+// the JSX transform converts to React, so we need to add it back in
+this['React'] = React;
+
+function A(props) {
+  return <div>Hello {props.x.b}</div>;
+}
+
+function B() {
+  return <div>World</div>;
+}
+
+function C() {
+  return "!";
+}
+
+function App() {
+  var x = { a: 1, b: "World" };
+
+  delete x.a;
+
+  return (
+    <div>
+      <A x={x} />
+      <B />
+      <C />
+    </div>
+  );
+}
+
+App.getTrials = function(renderer, Root) {
+  renderer.update(<Root />);
+  return [['simple render with delete on props key', renderer.toJSON()]];
+};
+
+if (this.__registerReactComponentRoot) {
+  __registerReactComponentRoot(App);
+}
+
+module.exports = App;