Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove __self and __source location from elements #28265

Merged
merged 2 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
`);
hoxyq marked this conversation as resolved.
Show resolved Hide resolved

await actAsync(
async () =>
Expand Down
5 changes: 0 additions & 5 deletions packages/react-devtools-shared/src/backend/legacy/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -851,9 +849,6 @@ export function attach(
// List of owners
owners,

// Location of component in source code.
source,

rootType: null,
rendererPackageName: null,
rendererVersion: null,
Expand Down
27 changes: 10 additions & 17 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -3132,7 +3132,6 @@ export function attach(

const {
_debugOwner,
_debugSource,
stateNode,
key,
memoizedProps,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.',
Expand Down
4 changes: 0 additions & 4 deletions packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 1 addition & 2 deletions packages/react-devtools-shared/src/backendAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ export function convertInspectedElementBackendToFrontend(
canViewSource,
hasLegacyContext,
id,
source,
type,
owners,
context,
Expand Down Expand Up @@ -261,7 +260,7 @@ export function convertInspectedElementBackendToFrontend(
rendererPackageName,
rendererVersion,
rootType,
source,
source: null, // TODO: Load source location lazily.
type,
owners:
owners === null
Expand Down
3 changes: 1 addition & 2 deletions packages/react-devtools-shared/src/frontend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* Be mindful of backwards compatibility when making changes.
*/

import type {Source} from 'shared/ReactElementType';
import type {
Dehydrated,
Unserializable,
Expand Down Expand Up @@ -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,

Expand Down
12 changes: 10 additions & 2 deletions packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
]);
});

Expand All @@ -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',
]);
});
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ describe('ReactFunctionComponent', () => {
return <FunctionComponent name="A" ref={() => {}} />;
}
}
Object.defineProperty(AnonymousParentUsingJSX, 'name', {value: undefined});

let instance1;

Expand All @@ -293,9 +292,6 @@ describe('ReactFunctionComponent', () => {
});
}
}
Object.defineProperty(AnonymousParentNotUsingJSX, 'name', {
value: undefined,
});

let instance2;
expect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -98,7 +97,6 @@ function getInspectorDataForInstance(
hierarchy: [],
props: emptyObject,
selectedIndex: null,
source: null,
};
}

Expand All @@ -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,
};
}

Expand Down
7 changes: 0 additions & 7 deletions packages/react-native-renderer/src/ReactNativeTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<{
Expand All @@ -165,7 +159,6 @@ export type InspectorData = $ReadOnly<{
}>,
selectedIndex: ?number,
props: InspectorDataProps,
source: ?InspectorDataSource,
}>;

export type TouchedViewDataAtPoint = $ReadOnly<{
Expand Down
11 changes: 0 additions & 11 deletions packages/react-reconciler/src/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Loading