Keep element.ref for now but warn on access

In the last commit I removed the `ref` property from the element type
completely. Instead, let's keep it for another release cycle but warn
if it's accessed.

In dev, we add a non-enumerable getter with `defineProperty` and warn
whenever it's invoked.

We don't warn on access if a ref is not given. This reduces false
positives in cases where a test serializer uses
`getOwnPropertyDescriptors`` to compare objects, like Jest does, which
is a problem because it bypasses non-enumerability.

So unfortunately this will trigger a false positive warning in Jest when
the diff is printed:

  expect(<div ref={ref} />).toEqual(<span ref={ref} />);

A bit sketchy, but this is what we've done for the `props.key` and
`props.ref` accessors for years, which implies it will be good enough
for `element.ref`, too. Let's see if anyone complains.

packages/jest-react/src/JestReact.js

@@ -40,15 +40,20 @@ function assertYieldsWereCleared(root) {
 }
 
 function createJSXElementForTestComparison(type, props) {
-  if (enableRefAsProp) {
-    return {
+  if (__DEV__ && enableRefAsProp) {
+    const element = {
       $$typeof: REACT_ELEMENT_TYPE,
       type: type,
       key: null,
       props: props,
       _owner: null,
       _store: __DEV__ ? {} : undefined,
     };
+    Object.defineProperty(element, 'ref', {
+      enumerable: false,
+      value: null,
+    });
+    return element;
   } else {
     return {
       $$typeof: REACT_ELEMENT_TYPE,

packages/react-client/src/ReactFlightClient.js

@@ -472,30 +472,34 @@ function createElement(
   key: mixed,
   props: mixed,
 ): React$Element<any> {
-  const element: any = enableRefAsProp
-    ? {
-        // This tag allows us to uniquely identify this as a React Element
-        $$typeof: REACT_ELEMENT_TYPE,
-
-        // Built-in properties that belong on the element
-        type,
-        key,
-        props,
-
-        // Record the component responsible for creating this element.
-        _owner: null,
-      }
-    : {
-        // Old behavior. When enableRefAsProp is off, `ref` is an extra field.
-        ref: null,
-
-        // Everything else is the same.
-        $$typeof: REACT_ELEMENT_TYPE,
-        type,
-        key,
-        props,
-        _owner: null,
-      };
+  let element: any;
+  if (__DEV__ && enableRefAsProp) {
+    // `ref` is non-enumerable in dev
+    element = ({
+      $$typeof: REACT_ELEMENT_TYPE,
+      type,
+      key,
+      props,
+      _owner: null,
+    }: any);
+    Object.defineProperty(element, 'ref', {
+      enumerable: false,
+      value: null,
+    });
+  } else {
+    element = ({
+      // This tag allows us to uniquely identify this as a React Element
+      $$typeof: REACT_ELEMENT_TYPE,
+
+      type,
+      key,
+      ref: null,
+      props,
+
+      // Record the component responsible for creating this element.
+      _owner: null,
+    }: any);
+  }
 
   if (__DEV__) {
     // We don't really need to add any of these but keeping them for good measure.

packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js

@@ -109,7 +109,6 @@ describe('ReactDeprecationWarnings', () => {
     await waitForAll([]);
   });
 
-  // @gate !enableRefAsProp || !__DEV__
   it('should warn when owner and self are different for string refs', async () => {
     class RefComponent extends React.Component {
       render() {
@@ -141,7 +140,6 @@ describe('ReactDeprecationWarnings', () => {
   });
 
   if (__DEV__) {
-    // @gate !enableRefAsProp
     it('should warn when owner and self are different for string refs', async () => {
       class RefComponent extends React.Component {
         render() {

packages/react-noop-renderer/src/createReactNoop.js

@@ -783,15 +783,20 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
   let currentEventPriority = DefaultEventPriority;
 
   function createJSXElementForTestComparison(type, props) {
-    if (enableRefAsProp) {
-      return {
-        $$typeof: REACT_ELEMENT_TYPE,
+    if (__DEV__ && enableRefAsProp) {
+      const element = {
         type: type,
+        $$typeof: REACT_ELEMENT_TYPE,
         key: null,
         props: props,
         _owner: null,
         _store: __DEV__ ? {} : undefined,
       };
+      Object.defineProperty(element, 'ref', {
+        enumerable: false,
+        value: null,
+      });
+      return element;
     } else {
       return {
         $$typeof: REACT_ELEMENT_TYPE,

packages/react/src/__tests__/ReactCreateElement-test.js

@@ -38,7 +38,7 @@ describe('ReactCreateElement', () => {
     expect(element.type).toBe(ComponentClass);
     expect(element.key).toBe(null);
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(element.ref).toBe(undefined);
+      expect(element.ref).toBe(null);
     } else {
       expect(element.ref).toBe(null);
     }
@@ -125,7 +125,7 @@ describe('ReactCreateElement', () => {
     expect(element.type).toBe('div');
     expect(element.key).toBe(null);
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(element.ref).toBe(undefined);
+      expect(element.ref).toBe(null);
     } else {
       expect(element.ref).toBe(null);
     }
@@ -180,7 +180,10 @@ describe('ReactCreateElement', () => {
     });
     expect(element.type).toBe(ComponentClass);
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(element.ref).toBe(undefined);
+      expect(() => expect(element.ref).toBe(ref)).toErrorDev(
+        'Accessing element.ref is no longer supported',
+        {withoutStack: true},
+      );
       const expectation = {foo: '56', ref};
       Object.freeze(expectation);
       expect(element.props).toEqual(expectation);
@@ -216,7 +219,7 @@ describe('ReactCreateElement', () => {
     expect(element.type).toBe(ComponentClass);
     expect(element.key).toBe(null);
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(element.ref).toBe(undefined);
+      expect(element.ref).toBe(null);
     } else {
       expect(element.ref).toBe(null);
     }
@@ -232,7 +235,7 @@ describe('ReactCreateElement', () => {
     const elementB = React.createElement('div', elementA.props);
     expect(elementB.key).toBe(null);
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(elementB.ref).toBe(undefined);
+      expect(elementB.ref).toBe(null);
     } else {
       expect(elementB.ref).toBe(null);
     }
@@ -246,7 +249,7 @@ describe('ReactCreateElement', () => {
     expect(element.type).toBe(ComponentClass);
     expect(element.key).toBe('12');
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(element.ref).toBe(undefined);
+      expect(element.ref).toBe(null);
     } else {
       expect(element.ref).toBe(null);
     }

packages/react/src/__tests__/ReactElementClone-test.js

@@ -18,6 +18,8 @@ describe('ReactElementClone', () => {
   let ComponentClass;
 
   beforeEach(() => {
+    jest.resetModules();
+
     act = require('internal-test-utils').act;
 
     PropTypes = require('prop-types');
@@ -373,7 +375,7 @@ describe('ReactElementClone', () => {
     const elementB = React.cloneElement(elementA, elementA.props);
     expect(elementB.key).toBe(null);
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(elementB.ref).toBe(undefined);
+      expect(elementB.ref).toBe(null);
     } else {
       expect(elementB.ref).toBe(null);
     }
@@ -395,6 +397,10 @@ describe('ReactElementClone', () => {
     expect(clone.key).toBe('12');
     if (gate(flags => flags.enableRefAsProp)) {
       expect(clone.props.ref).toBe('34');
+      expect(() => expect(clone.ref).toBe('34')).toErrorDev(
+        'Accessing element.ref is no longer supported',
+        {withoutStack: true},
+      );
       expect(clone.props).toEqual({foo: 'ef', ref: '34'});
     } else {
       expect(clone.ref).toBe('34');
@@ -421,8 +427,7 @@ describe('ReactElementClone', () => {
     expect(clone.type).toBe(ComponentClass);
     expect(clone.key).toBe('null');
     if (gate(flags => flags.enableRefAsProp)) {
-      // TODO: Remove `ref` field from the element entirely.
-      expect(clone.ref).toBe(undefined);
+      expect(clone.ref).toBe(null);
       expect(clone.props).toEqual({foo: 'ef', ref: null});
     } else {
       expect(clone.ref).toBe(null);

packages/react/src/__tests__/ReactJSXTransformIntegration-test.js

@@ -56,7 +56,7 @@ describe('ReactJSXTransformIntegration', () => {
     expect(element.type).toBe(Component);
     expect(element.key).toBe(null);
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(element.ref).toBe(undefined);
+      expect(element.ref).toBe(null);
     } else {
       expect(element.ref).toBe(null);
     }
@@ -70,7 +70,7 @@ describe('ReactJSXTransformIntegration', () => {
     expect(element.type).toBe('div');
     expect(element.key).toBe(null);
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(element.ref).toBe(undefined);
+      expect(element.ref).toBe(null);
     } else {
       expect(element.ref).toBe(null);
     }
@@ -85,7 +85,7 @@ describe('ReactJSXTransformIntegration', () => {
     expect(element.type).toBe('div');
     expect(element.key).toBe(null);
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(element.ref).toBe(undefined);
+      expect(element.ref).toBe(null);
     } else {
       expect(element.ref).toBe(null);
     }
@@ -125,7 +125,10 @@ describe('ReactJSXTransformIntegration', () => {
     const element = <Component ref={ref} foo="56" />;
     expect(element.type).toBe(Component);
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(element.ref).toBe(undefined);
+      expect(() => expect(element.ref).toBe(ref)).toErrorDev(
+        'Accessing element.ref is no longer supported',
+        {withoutStack: true},
+      );
       const expectation = {foo: '56', ref};
       Object.freeze(expectation);
       expect(element.props).toEqual(expectation);
@@ -142,7 +145,7 @@ describe('ReactJSXTransformIntegration', () => {
     expect(element.type).toBe(Component);
     expect(element.key).toBe('12');
     if (gate(flags => flags.enableRefAsProp)) {
-      expect(element.ref).toBe(undefined);
+      expect(element.ref).toBe(null);
     } else {
       expect(element.ref).toBe(null);
     }