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

Prefer JSX in ReactNoop assertions (to combat out-of-memory test runs) #26127

Merged
merged 1 commit into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions packages/react-noop-renderer/src/ReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import createReactNoop from './createReactNoop';
export const {
_Scheduler,
getChildren,
dangerouslyGetChildren,
getPendingChildren,
dangerouslyGetPendingChildren,
getOrCreateRootContainer,
createRoot,
createLegacyRoot,
Expand Down
2 changes: 2 additions & 0 deletions packages/react-noop-renderer/src/ReactNoopPersistent.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import createReactNoop from './createReactNoop';
export const {
_Scheduler,
getChildren,
dangerouslyGetChildren,
getPendingChildren,
dangerouslyGetPendingChildren,
getOrCreateRootContainer,
createRoot,
createLegacyRoot,
Expand Down
26 changes: 25 additions & 1 deletion packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -789,11 +789,35 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
_Scheduler: Scheduler,

getChildren(rootID: string = DEFAULT_ROOT_ID) {
throw new Error(
'No longer supported due to bad performance when used with `expect()`. ' +
'Use `ReactNoop.getChildrenAsJSX()` instead or, if you really need to, `dangerouslyGetChildren` after you carefully considered the warning in its JSDOC.',
);
},

getPendingChildren(rootID: string = DEFAULT_ROOT_ID) {
throw new Error(
'No longer supported due to bad performance when used with `expect()`. ' +
'Use `ReactNoop.getPendingChildrenAsJSX()` instead or, if you really need to, `dangerouslyGetPendingChildren` after you carefully considered the warning in its JSDOC.',
);
},

/**
* Prefer using `getChildrenAsJSX`.
* Using the returned children in `.toEqual` has very poor performance on mismatch due to deep equality checking of fiber structures.
* Make sure you deeply remove enumerable properties before passing it to `.toEqual`, or, better, use `getChildrenAsJSX` or `toMatchRenderedOutput`.
*/
dangerouslyGetChildren(rootID: string = DEFAULT_ROOT_ID) {
const container = rootContainers.get(rootID);
return getChildren(container);
},

getPendingChildren(rootID: string = DEFAULT_ROOT_ID) {
/**
* Prefer using `getPendingChildrenAsJSX`.
* Using the returned children in `.toEqual` has very poor performance on mismatch due to deep equality checking of fiber structures.
* Make sure you deeply remove enumerable properties before passing it to `.toEqual`, or, better, use `getChildrenAsJSX` or `toMatchRenderedOutput`.
*/
dangerouslyGetPendingChildren(rootID: string = DEFAULT_ROOT_ID) {
const container = rootContainers.get(rootID);
return getPendingChildren(container);
},
Expand Down
24 changes: 10 additions & 14 deletions packages/react-reconciler/src/__tests__/ReactExpiration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,6 @@ describe('ReactExpiration', () => {
}
}

function span(prop) {
return {type: 'span', children: [], prop, hidden: false};
}

function flushNextRenderIfExpired() {
// This will start rendering the next level of work. If the work hasn't
// expired yet, React will exit without doing anything. If it has expired,
Expand All @@ -127,21 +123,21 @@ describe('ReactExpiration', () => {
ReactNoop.render(<span prop="done" />);
}

expect(ReactNoop.getChildren()).toEqual([]);
expect(ReactNoop).toMatchRenderedOutput(null);

// Nothing has expired yet because time hasn't advanced.
flushNextRenderIfExpired();
expect(ReactNoop.getChildren()).toEqual([]);
expect(ReactNoop).toMatchRenderedOutput(null);

// Advance time a bit, but not enough to expire the low pri update.
ReactNoop.expire(4500);
flushNextRenderIfExpired();
expect(ReactNoop.getChildren()).toEqual([]);
expect(ReactNoop).toMatchRenderedOutput(null);

// Advance by another second. Now the update should expire and flush.
ReactNoop.expire(500);
flushNextRenderIfExpired();
expect(ReactNoop.getChildren()).toEqual([span('done')]);
expect(ReactNoop).toMatchRenderedOutput(<span prop="done" />);
});

it('two updates of like priority in the same event always flush within the same batch', () => {
Expand Down Expand Up @@ -181,20 +177,20 @@ describe('ReactExpiration', () => {

// Don't advance time by enough to expire the first update.
expect(Scheduler).toHaveYielded([]);
expect(ReactNoop.getChildren()).toEqual([]);
expect(ReactNoop).toMatchRenderedOutput(null);

// Schedule another update.
ReactNoop.render(<TextClass text="B" />);
// Both updates are batched
expect(Scheduler).toFlushAndYield(['B [render]', 'B [commit]']);
expect(ReactNoop.getChildren()).toEqual([span('B')]);
expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);

// Now do the same thing again, except this time don't flush any work in
// between the two updates.
ReactNoop.render(<TextClass text="A" />);
Scheduler.unstable_advanceTime(2000);
expect(Scheduler).toHaveYielded([]);
expect(ReactNoop.getChildren()).toEqual([span('B')]);
expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
// Schedule another update.
ReactNoop.render(<TextClass text="B" />);
// The updates should flush in the same batch, since as far as the scheduler
Expand Down Expand Up @@ -242,20 +238,20 @@ describe('ReactExpiration', () => {

// Don't advance time by enough to expire the first update.
expect(Scheduler).toHaveYielded([]);
expect(ReactNoop.getChildren()).toEqual([]);
expect(ReactNoop).toMatchRenderedOutput(null);

// Schedule another update.
ReactNoop.render(<TextClass text="B" />);
// Both updates are batched
expect(Scheduler).toFlushAndYield(['B [render]', 'B [commit]']);
expect(ReactNoop.getChildren()).toEqual([span('B')]);
expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);

// Now do the same thing again, except this time don't flush any work in
// between the two updates.
ReactNoop.render(<TextClass text="A" />);
Scheduler.unstable_advanceTime(2000);
expect(Scheduler).toHaveYielded([]);
expect(ReactNoop.getChildren()).toEqual([span('B')]);
expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);

// Perform some synchronous work. The scheduler must assume we're inside
// the same event.
Expand Down
Loading