Remove __self and __source location from elements

Along with all the places using it like the _debugSource on Fiber. This removes source location info from React DevTools and React Native Inspector.
facebook · Feb 7, 2024 · 6bb9cb4 · 6bb9cb4
1 parent a1ace9d
commit 6bb9cb4
Show file tree

Hide file tree

Showing 26 changed files with 60 additions and 211 deletions.
diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js
@@ -475,18 +475,6 @@ function createElement(
       writable: true,
       value: true, // This element has already been validated on the server.
     });
-    Object.defineProperty(element, '_self', {
-      configurable: false,
-      enumerable: false,
-      writable: false,
-      value: null,
-    });
-    Object.defineProperty(element, '_source', {
-      configurable: false,
-      enumerable: false,
-      writable: false,
-      value: null,
-    });
   }
   return element;
 }

diff --git a/packages/react-devtools-inline/__tests__/__e2e__/components.test.js b/packages/react-devtools-inline/__tests__/__e2e__/components.test.js
@@ -92,14 +92,15 @@ test.describe('Components', () => {
           ? valueElement.value
           : valueElement.innerText;
 
-        return [name, value, source.innerText];
+        return [name, value, source ? source.innerText : null];
       },
       {name: isEditableName, value: isEditableValue}
     );
 
     expect(propName).toBe('label');
     expect(propValue).toBe('"one"');
-    expect(sourceText).toMatch(/ListApp[a-zA-Z]*\.js/);
+    expect(sourceText).toBe(null);
+    // TODO: expect(sourceText).toMatch(/ListApp[a-zA-Z]*\.js/);
   });
 
   test('should allow props to be edited', async () => {

diff --git a/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js b/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js
@@ -242,7 +242,13 @@ describe('Store component filters', () => {
         ]),
     );
 
-    expect(store).toMatchInlineSnapshot(`[root]`);
+    // TODO: Filtering should work on component location.
+    // expect(store).toMatchInlineSnapshot(`[root]`);
+    expect(store).toMatchInlineSnapshot(`
+      [root]
+        ▾ <Component>
+            <div>
+    `);
 
     await actAsync(
       async () =>

diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js
@@ -773,12 +773,10 @@ export function attach(
     let owners = null;
     let props = null;
     let state = null;
-    let source = null;
 
     const element = internalInstance._currentElement;
     if (element !== null) {
       props = element.props;
-      source = element._source != null ? element._source : null;
 
       let owner = element._owner;
       if (owner) {
@@ -851,9 +849,6 @@ export function attach(
       // List of owners
       owners,
 
-      // Location of component in source code.
-      source,
-
       rootType: null,
       rendererPackageName: null,
       rendererVersion: null,

diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js
@@ -958,7 +958,7 @@ export function attach(
 
   // NOTICE Keep in sync with get*ForFiber methods
   function shouldFilterFiber(fiber: Fiber): boolean {
-    const {_debugSource, tag, type, key} = fiber;
+    const {tag, type, key} = fiber;
 
     switch (tag) {
       case DehydratedSuspenseComponent:
@@ -1010,15 +1010,15 @@ export function attach(
       }
     }
 
-    if (_debugSource != null && hideElementsWithPaths.size > 0) {
-      const {fileName} = _debugSource;
-      // eslint-disable-next-line no-for-of-loops/no-for-of-loops
-      for (const pathRegExp of hideElementsWithPaths) {
-        if (pathRegExp.test(fileName)) {
-          return true;
-        }
-      }
-    }
+    // TODO: Figure out a way to filter by path in the new model which has no debug info.
+    // if (hideElementsWithPaths.size > 0) {
+    //  const {fileName} = ...;
+    //  for (const pathRegExp of hideElementsWithPaths) {
+    //    if (pathRegExp.test(fileName)) {
+    //      return true;
+    //    }
+    //  }
+    // }
 
     return false;
   }
@@ -3132,7 +3132,6 @@ export function attach(
 
     const {
       _debugOwner,
-      _debugSource,
       stateNode,
       key,
       memoizedProps,
@@ -3362,9 +3361,6 @@ export function attach(
       // List of owners
       owners,
 
-      // Location of component in source code.
-      source: _debugSource || null,
-
       rootType,
       rendererPackageName: renderer.rendererPackageName,
       rendererVersion: renderer.version,
@@ -3725,9 +3721,6 @@ export function attach(
     if (nativeNodes !== null) {
       console.log('Nodes:', nativeNodes);
     }
-    if (result.source !== null) {
-      console.log('Location:', result.source);
-    }
     if (window.chrome || /firefox/i.test(navigator.userAgent)) {
       console.log(
         'Right-click any value to save it as a global variable for further inspection.',

diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js
@@ -15,7 +15,6 @@
  */
 
 import type {ReactContext, Wakeable} from 'shared/ReactTypes';
-import type {Source} from 'shared/ReactElementType';
 import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
 import type {
   ComponentFilter,
@@ -280,9 +279,6 @@ export type InspectedElement = {
   // List of owners
   owners: Array<SerializedElement> | null,
 
-  // Location of component in source code.
-  source: Source | null,
-
   type: ElementType,
 
   // Meta information about the root this element belongs to.

diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js
@@ -226,7 +226,6 @@ export function convertInspectedElementBackendToFrontend(
     canViewSource,
     hasLegacyContext,
     id,
-    source,
     type,
     owners,
     context,
@@ -261,7 +260,7 @@ export function convertInspectedElementBackendToFrontend(
     rendererPackageName,
     rendererVersion,
     rootType,
-    source,
+    source: null, // TODO: Load source location lazily.
     type,
     owners:
       owners === null

diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js
@@ -14,7 +14,6 @@
  * Be mindful of backwards compatibility when making changes.
  */
 
-import type {Source} from 'shared/ReactElementType';
 import type {
   Dehydrated,
   Unserializable,
@@ -220,7 +219,7 @@ export type InspectedElement = {
   owners: Array<SerializedElement> | null,
 
   // Location of component in source code.
-  source: Source | null,
+  source: null, // TODO: Reinstate a way to load this lazily.
 
   type: ElementType,
 

diff --git a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js
@@ -131,6 +131,10 @@ describe('ReactDeprecationWarnings', () => {
         'We ask you to manually fix this case by using useRef() or createRef() instead. ' +
         'Learn more about using refs safely here: ' +
         'https://reactjs.org/link/strict-mode-string-ref',
+      'Warning: Component "Component" contains the string ref "refComponent". ' +
+        'Support for string refs will be removed in a future major release. We recommend ' +
+        'using useRef() or createRef() instead. Learn more about using refs safely here: ' +
+        'https://reactjs.org/link/strict-mode-string-ref',
     ]);
   });
 
@@ -155,14 +159,18 @@ describe('ReactDeprecationWarnings', () => {
       }
 
       ReactNoop.render(<Component />);
-      await expect(async () => await waitForAll([])).toErrorDev(
+      await expect(async () => await waitForAll([])).toErrorDev([
         'Warning: Component "Component" contains the string ref "refComponent". ' +
           'Support for string refs will be removed in a future major release. ' +
           'This case cannot be automatically converted to an arrow function. ' +
           'We ask you to manually fix this case by using useRef() or createRef() instead. ' +
           'Learn more about using refs safely here: ' +
           'https://reactjs.org/link/strict-mode-string-ref',
-      );
+        'Warning: Component "Component" contains the string ref "refComponent". ' +
+          'Support for string refs will be removed in a future major release. We recommend ' +
+          'using useRef() or createRef() instead. Learn more about using refs safely here: ' +
+          'https://reactjs.org/link/strict-mode-string-ref',
+      ]);
     });
   }
 });
diff --git a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js
@@ -270,7 +270,6 @@ describe('ReactFunctionComponent', () => {
         return <FunctionComponent name="A" ref={() => {}} />;
       }
     }
-    Object.defineProperty(AnonymousParentUsingJSX, 'name', {value: undefined});
 
     let instance1;
 
@@ -293,9 +292,6 @@ describe('ReactFunctionComponent', () => {
         });
       }
     }
-    Object.defineProperty(AnonymousParentNotUsingJSX, 'name', {
-      value: undefined,
-    });
 
     let instance2;
     expect(() => {

diff --git a/packages/react-native-renderer/src/ReactNativeFiberInspector.js b/packages/react-native-renderer/src/ReactNativeFiberInspector.js
@@ -37,7 +37,6 @@ function createHierarchy(fiberHierarchy) {
     getInspectorData: findNodeHandle => {
       return {
         props: getHostProps(fiber),
-        source: fiber._debugSource,
         measure: callback => {
           // If this is Fabric, we'll find a shadow node and use that to measure.
           const hostFiber = findCurrentHostFiber(fiber);
@@ -98,7 +97,6 @@ function getInspectorDataForInstance(
         hierarchy: [],
         props: emptyObject,
         selectedIndex: null,
-        source: null,
       };
     }
 
@@ -107,15 +105,13 @@ function getInspectorDataForInstance(
     const instance = lastNonHostInstance(fiberHierarchy);
     const hierarchy = createHierarchy(fiberHierarchy);
     const props = getHostProps(instance);
-    const source = instance._debugSource;
     const selectedIndex = fiberHierarchy.indexOf(instance);
 
     return {
       closestInstance: instance,
       hierarchy,
       props,
       selectedIndex,
-      source,
     };
   }
 

diff --git a/packages/react-native-renderer/src/ReactNativeTypes.js b/packages/react-native-renderer/src/ReactNativeTypes.js
@@ -142,19 +142,13 @@ type InspectorDataProps = $ReadOnly<{
   ...
 }>;
 
-type InspectorDataSource = $ReadOnly<{
-  fileName?: string,
-  lineNumber?: number,
-}>;
-
 type InspectorDataGetter = (
   <TElementType: ElementType>(
     componentOrHandle: ElementRef<TElementType> | number,
   ) => ?number,
 ) => $ReadOnly<{
   measure: (callback: MeasureOnSuccessCallback) => void,
   props: InspectorDataProps,
-  source: InspectorDataSource,
 }>;
 
 export type InspectorData = $ReadOnly<{
@@ -165,7 +159,6 @@ export type InspectorData = $ReadOnly<{
   }>,
   selectedIndex: ?number,
   props: InspectorDataProps,
-  source: ?InspectorDataSource,
 }>;
 
 export type TouchedViewDataAtPoint = $ReadOnly<{

diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js
@@ -129,14 +129,6 @@ function coerceRef(
   ) {
     if (__DEV__) {
       if (
-        // We warn in ReactElement.js if owner and self are equal for string refs
-        // because these cannot be automatically converted to an arrow function
-        // using a codemod. Therefore, we don't have to warn about string refs again.
-        !(
-          element._owner &&
-          element._self &&
-          element._owner.stateNode !== element._self
-        ) &&
         // Will already throw with "Function components cannot have string refs"
         !(
           element._owner &&
@@ -446,7 +438,6 @@ function createChildReconciler(
         existing.ref = coerceRef(returnFiber, current, element);
         existing.return = returnFiber;
         if (__DEV__) {
-          existing._debugSource = element._source;
           existing._debugOwner = element._owner;
         }
         return existing;
@@ -1234,7 +1225,6 @@ function createChildReconciler(
             const existing = useFiber(child, element.props.children);
             existing.return = returnFiber;
             if (__DEV__) {
-              existing._debugSource = element._source;
               existing._debugOwner = element._owner;
             }
             return existing;
@@ -1260,7 +1250,6 @@ function createChildReconciler(
             existing.ref = coerceRef(returnFiber, child, element);
             existing.return = returnFiber;
             if (__DEV__) {
-              existing._debugSource = element._source;
               existing._debugOwner = element._owner;
             }
             return existing;