Skip to content

Fix some DevTools regression test actions and assertions #34459

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Sep 15, 2025
Merged

Fix some DevTools regression test actions and assertions #34459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b08fe8f
Fix DevTools regression test actions and assertions
eps1lon Sep 10, 2025
d2037a0
Passing React 17
eps1lon Sep 10, 2025
496a618
Passing React 16.5
eps1lon Sep 10, 2025
aba9379
Paranoia about JSX runtime
eps1lon Sep 10, 2025
f4e1b8c
Fix e2e regression tests
eps1lon Sep 10, 2025
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
4 changes: 3 additions & 1 deletion packages/react-devtools-inline/__tests__/__e2e__/components.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ test.describe('Components', () => {

const name = isEditable.name
? existingNameElements[0].value
: existingNameElements[0].innerText;
: existingNameElements[0].innerText
// remove trailing colon
.slice(0, -1);
Comment on lines +96 to +98
Copy link
Collaborator Author

@eps1lon eps1lon Sep 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accounts for https://github.com/facebook/react/pull/34095/files#diff-94a124a1b825898267d7c9f43ef03cafaf2f55da968b1bd46d1f5ffea9def7bfR233.

Didn't think it's worth it to add even more test-specific markup

const value = isEditable.value
? existingValueElements[0].value
: existingValueElements[0].innerText;
Expand Down
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/__tests__/__serializers__/storeSerializer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ export function test(maybeStore) {
}

// print() is part of Jest's serializer API
export function print(store, serialize, indent) {
return printStore(store);
export function print(store, serialize, indent, includeSuspense = true) {
return printStore(store, false, null, includeSuspense);
}

// Used for Jest snapshot testing.
Expand Down
93 changes: 65 additions & 28 deletions packages/react-devtools-shared/src/__tests__/profilingCache-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,34 +724,69 @@ describe('ProfilingCache', () => {
const rootID = store.roots[0];
const commitData = store.profilerStore.getDataForRoot(rootID).commitData;
expect(commitData).toHaveLength(2);
expect(commitData[0].fiberActualDurations).toMatchInlineSnapshot(`
Map {
1 => 15,
2 => 15,
3 => 5,
4 => 2,
}
`);
expect(commitData[0].fiberSelfDurations).toMatchInlineSnapshot(`
Map {
1 => 0,
2 => 10,
3 => 3,
4 => 2,
}
`);
expect(commitData[1].fiberActualDurations).toMatchInlineSnapshot(`
Map {
5 => 3,
3 => 3,
}
`);
expect(commitData[1].fiberSelfDurations).toMatchInlineSnapshot(`
Map {
5 => 3,
3 => 0,
}
`);

const isLegacySuspense = React.version.startsWith('17');
if (isLegacySuspense) {
expect(commitData[0].fiberActualDurations).toMatchInlineSnapshot(`
Map {
1 => 15,
2 => 15,
3 => 5,
4 => 3,
5 => 2,
Comment on lines +728 to +736
Copy link
Collaborator Author

@eps1lon eps1lon Sep 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to bisect where the additional node comes from. I suspect something legacy Suspense related similar to the other snapshot tests that had a <Lazy> show up.

Copy link
Collaborator Author

@eps1lon eps1lon Sep 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in 5bbf9be (#34062)

}
`);
expect(commitData[0].fiberSelfDurations).toMatchInlineSnapshot(`
Map {
1 => 0,
2 => 10,
3 => 3,
4 => 3,
5 => 2,
}
`);
expect(commitData[1].fiberActualDurations).toMatchInlineSnapshot(`
Map {
6 => 3,
3 => 3,
}
`);
expect(commitData[1].fiberSelfDurations).toMatchInlineSnapshot(`
Map {
6 => 3,
3 => 0,
}
`);
} else {
expect(commitData[0].fiberActualDurations).toMatchInlineSnapshot(`
Map {
1 => 15,
2 => 15,
3 => 5,
4 => 2,
}
`);
expect(commitData[0].fiberSelfDurations).toMatchInlineSnapshot(`
Map {
1 => 0,
2 => 10,
3 => 3,
4 => 2,
}
`);
expect(commitData[1].fiberActualDurations).toMatchInlineSnapshot(`
Map {
5 => 3,
3 => 3,
}
`);
expect(commitData[1].fiberSelfDurations).toMatchInlineSnapshot(`
Map {
5 => 3,
3 => 0,
}
`);
}
});

// @reactVersion >= 16.9
Expand Down Expand Up @@ -866,6 +901,7 @@ describe('ProfilingCache', () => {
"hocDisplayNames": null,
"id": 1,
"key": null,
"stack": null,
"type": 11,
},
],
Expand Down Expand Up @@ -908,6 +944,7 @@ describe('ProfilingCache', () => {
"hocDisplayNames": null,
"id": 1,
"key": null,
"stack": null,
"type": 11,
},
],
Expand Down
68 changes: 55 additions & 13 deletions packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ import {
} from './utils';

describe('commit tree', () => {
let React;
let React = require('react');
let Scheduler;
let store: Store;
let utils;
const isLegacySuspense =
React.version.startsWith('16') || React.version.startsWith('17');

beforeEach(() => {
utils = require('./utils');
Expand Down Expand Up @@ -184,17 +186,32 @@ describe('commit tree', () => {
utils.act(() => store.profilerStore.startProfiling());
utils.act(() => legacyRender(<App renderChildren={true} />));
await Promise.resolve();
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
<Suspense>
`);
if (isLegacySuspense) {
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
▾ <Suspense>
<Lazy>
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
} else {
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
<Suspense>
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
}
utils.act(() => legacyRender(<App renderChildren={true} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
▾ <Suspense>
<LazyInnerComponent>
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
utils.act(() => legacyRender(<App renderChildren={false} />));
expect(store).toMatchInlineSnapshot(`
Expand All @@ -214,7 +231,13 @@ describe('commit tree', () => {
);
}

expect(commitTrees[0].nodes.size).toBe(3); // <Root> + <App> + <Suspense>
expect(commitTrees[0].nodes.size).toBe(
isLegacySuspense
? // <Root> + <App> + <Suspense> + <Lazy>
4
: // <Root> + <App> + <Suspense>
3,
);
expect(commitTrees[1].nodes.size).toBe(4); // <Root> + <App> + <Suspense> + <LazyInnerComponent>
expect(commitTrees[2].nodes.size).toBe(2); // <Root> + <App>
});
Expand Down Expand Up @@ -268,11 +291,24 @@ describe('commit tree', () => {
it('should support Lazy components that are unmounted before resolving (legacy render)', async () => {
utils.act(() => store.profilerStore.startProfiling());
utils.act(() => legacyRender(<App renderChildren={true} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
<Suspense>
`);
if (isLegacySuspense) {
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
▾ <Suspense>
<Lazy>
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
} else {
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
<Suspense>
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
}
utils.act(() => legacyRender(<App renderChildren={false} />));
expect(store).toMatchInlineSnapshot(`
[root]
Expand All @@ -291,7 +327,13 @@ describe('commit tree', () => {
);
}

expect(commitTrees[0].nodes.size).toBe(3); // <Root> + <App> + <Suspense>
expect(commitTrees[0].nodes.size).toBe(
isLegacySuspense
? // <Root> + <App> + <Suspense> + <Lazy>
4
: // <Root> + <App> + <Suspense>
3,
);
expect(commitTrees[1].nodes.size).toBe(2); // <Root> + <App>
});

Expand Down
69 changes: 42 additions & 27 deletions packages/react-devtools-shared/src/__tests__/store-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,35 @@ describe('Store', () => {
let store;
let withErrorsOrWarningsIgnored;

function readValue(promise) {
if (typeof React.use === 'function') {
return React.use(promise);
}

// Support for React < 19.0
switch (promise.status) {
case 'fulfilled':
return promise.value;
case 'rejected':
throw promise.reason;
case 'pending':
throw promise;
default:
promise.status = 'pending';
promise.then(
value => {
promise.status = 'fulfilled';
promise.value = value;
},
reason => {
promise.status = 'rejected';
promise.reason = reason;
},
);
throw promise;
}
}

beforeAll(() => {
// JSDDOM doesn't implement getClientRects so we're just faking one for testing purposes
Element.prototype.getClientRects = function (this: Element) {
Expand Down Expand Up @@ -107,11 +136,7 @@ describe('Store', () => {
let Dynamic = null;
const Owner = () => {
Dynamic = <Child />;
if (React.use) {
React.use(promise);
} else {
throw promise;
}
readValue(promise);
};
const Parent = () => {
return Dynamic;
Expand Down Expand Up @@ -462,12 +487,9 @@ describe('Store', () => {
// @reactVersion >= 18.0
it('should display Suspense nodes properly in various states', async () => {
const Loading = () => <div>Loading...</div>;
const never = new Promise(() => {});
const SuspendingComponent = () => {
if (React.use) {
React.use(new Promise(() => {}));
} else {
throw new Promise(() => {});
}
readValue(never);
};
const Component = () => {
return <div>Hello</div>;
Expand Down Expand Up @@ -514,12 +536,9 @@ describe('Store', () => {
it('should support nested Suspense nodes', async () => {
const Component = () => null;
const Loading = () => <div>Loading...</div>;
const never = new Promise(() => {});
const Never = () => {
if (React.use) {
React.use(new Promise(() => {}));
} else {
throw new Promise(() => {});
}
readValue(never);
};

const Wrapper = ({
Expand Down Expand Up @@ -1019,12 +1038,9 @@ describe('Store', () => {

it('should display a partially rendered SuspenseList', async () => {
const Loading = () => <div>Loading...</div>;
const never = new Promise(() => {});
const SuspendingComponent = () => {
if (React.use) {
React.use(new Promise(() => {}));
} else {
throw new Promise(() => {});
}
readValue(never);
};
const Component = () => {
return <div>Hello</div>;
Expand Down Expand Up @@ -1379,12 +1395,9 @@ describe('Store', () => {
// @reactVersion >= 18.0
it('should display Suspense nodes properly in various states', async () => {
const Loading = () => <div>Loading...</div>;
const never = new Promise(() => {});
const SuspendingComponent = () => {
if (React.use) {
React.use(new Promise(() => {}));
} else {
throw new Promise(() => {});
}
readValue(never);
};
const Component = () => {
return <div>Hello</div>;
Expand Down Expand Up @@ -2081,6 +2094,8 @@ describe('Store', () => {
[root]
▾ <App>
<Suspense>
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);

// Render again to unmount it before it finishes loading
Expand Down Expand Up @@ -2826,7 +2841,7 @@ describe('Store', () => {

function Component({children, promise}) {
if (promise) {
React.use(promise);
readValue(promise);
}
return <div>{children}</div>;
}
Expand Down Expand Up @@ -2901,7 +2916,7 @@ describe('Store', () => {

function Component({children, promise}) {
if (promise) {
React.use(promise);
readValue(promise);
}
return <div>{children}</div>;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ describe('Store component filters', () => {
`);
});

// @reactVersion >= 16.0
// @reactVersion >= 16.6
it('should filter Suspense', async () => {
Comment on lines +137 to 138
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<Suspense> was renamed from unstable_Placeholder in 16.6

const Suspense = React.Suspense;
await actAsync(async () =>
Expand Down
Loading
Loading