Skip to content

Commit

Permalink
Remove invokeGuardedCallback and replay trick
Browse files Browse the repository at this point in the history
  • Loading branch information
@sebmarkbage
sebmarkbage committed Mar 9, 2024
1 parent 966d174 commit de66616
Show file tree
Hide file tree
Showing 48 changed files with 209 additions and 1,251 deletions.
4 changes: 0 additions & 4 deletions packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
let ReactCache;
let createResource;
let React;
let ReactFeatureFlags;
let ReactNoop;
let Scheduler;
let Suspense;
Expand All @@ -27,9 +26,6 @@ describe('ReactCache', () => {
beforeEach(() => {
jest.resetModules();

ReactFeatureFlags = require('shared/ReactFeatureFlags');

ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
React = require('react');
Suspense = React.Suspense;
ReactCache = require('react-cache');
Expand Down
15 changes: 0 additions & 15 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1231,13 +1231,6 @@ describe('ReactCompositeComponent', () => {
});
}).toThrow();
}).toErrorDev([
// Expect two errors because invokeGuardedCallback will dispatch an error event,
// Causing the warning to be logged again.
'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' +
'did you accidentally return an object from the constructor?',
'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' +
'did you accidentally return an object from the constructor?',
// And then two more because we retry errors.
'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' +
'did you accidentally return an object from the constructor?',
'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' +
Expand Down Expand Up @@ -1278,14 +1271,6 @@ describe('ReactCompositeComponent', () => {
});
}).toThrow();
}).toErrorDev([
// Expect two errors because invokeGuardedCallback will dispatch an error event,
// Causing the warning to be logged again.
'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' +
'you may have forgotten to define `render`.',
'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' +
'you may have forgotten to define `render`.',

// And then two more because we retry errors.
'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' +
'you may have forgotten to define `render`.',
'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' +
Expand Down
219 changes: 25 additions & 194 deletions packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,64 +82,25 @@ describe('ReactDOMConsoleErrorReporting', () => {
);
});

if (__DEV__) {
expect(windowOnError.mock.calls).toEqual([
[
// Reported because we're in a browser click event:
expect.objectContaining({
message: 'Boom',
}),
],
[
// This one is jsdom-only. Real browser deduplicates it.
// (In DEV, we have a nested event due to guarded callback.)
expect.objectContaining({
message: 'Boom',
}),
],
]);
expect(console.error.mock.calls).toEqual([
[
// Reported because we're in a browser click event:
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
[
// This one is jsdom-only. Real browser deduplicates it.
// (In DEV, we have a nested event due to guarded callback.)
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
]);
} else {
expect(windowOnError.mock.calls).toEqual([
[
// Reported because we're in a browser click event:
expect.objectContaining({
expect(windowOnError.mock.calls).toEqual([
[
// Reported because we're in a browser click event:
expect.objectContaining({
message: 'Boom',
}),
],
]);
expect(console.error.mock.calls).toEqual([
[
// Reported because we're in a browser click event:
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
],
]);
expect(console.error.mock.calls).toEqual([
[
// Reported because we're in a browser click event:
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
]);
}
type: 'unhandled exception',
}),
],
]);

// Check next render doesn't throw.
windowOnError.mockReset();
Expand Down Expand Up @@ -168,41 +129,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
});

if (__DEV__) {
expect(windowOnError.mock.calls).toEqual([
[
// Reported due to guarded callback:
expect.objectContaining({
message: 'Boom',
}),
],
[
// This is only duplicated with createRoot
// because it retries once with a sync render.
expect.objectContaining({
message: 'Boom',
}),
],
]);
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Reported due to the guarded callback:
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
[
// This is only duplicated with createRoot
// because it retries once with a sync render.
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
[
// Addendum by React:
expect.stringContaining(
Expand Down Expand Up @@ -254,41 +182,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
});

if (__DEV__) {
expect(windowOnError.mock.calls).toEqual([
[
// Reported due to guarded callback:
expect.objectContaining({
message: 'Boom',
}),
],
[
// This is only duplicated with createRoot
// because it retries once with a sync render.
expect.objectContaining({
message: 'Boom',
}),
],
]);
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Reported by jsdom due to the guarded callback:
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
[
// This is only duplicated with createRoot
// because it retries once with a sync render.
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
[
// Addendum by React:
expect.stringContaining(
Expand Down Expand Up @@ -340,24 +235,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
});

if (__DEV__) {
expect(windowOnError.mock.calls).toEqual([
[
// Reported due to guarded callback:
expect.objectContaining({
message: 'Boom',
}),
],
]);
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Reported due to the guarded callback:
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
[
// Addendum by React:
expect.stringContaining(
Expand Down Expand Up @@ -412,24 +291,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
});

if (__DEV__) {
expect(windowOnError.mock.calls).toEqual([
[
// Reported due to guarded callback:
expect.objectContaining({
message: 'Boom',
}),
],
]);
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Reported by jsdom due to the guarded callback:
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
[
// Addendum by React:
expect.stringContaining(
Expand All @@ -438,7 +301,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
],
]);
} else {
// The top-level error was caught with try/catch, and there's no guarded callback,
// The top-level error was caught with try/catch,
// so in production we don't see an error event.
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
Expand Down Expand Up @@ -481,24 +344,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
});

if (__DEV__) {
expect(windowOnError.mock.calls).toEqual([
[
// Reported due to guarded callback:
expect.objectContaining({
message: 'Boom',
}),
],
]);
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Reported due to the guarded callback:
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
[
// Addendum by React:
expect.stringContaining(
Expand All @@ -507,7 +354,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
],
]);
} else {
// The top-level error was caught with try/catch, and there's no guarded callback,
// The top-level error was caught with try/catch,
// so in production we don't see an error event.
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
Expand Down Expand Up @@ -553,24 +400,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
});

if (__DEV__) {
expect(windowOnError.mock.calls).toEqual([
[
// Reported due to guarded callback:
expect.objectContaining({
message: 'Boom',
}),
],
]);
expect(windowOnError.mock.calls).toEqual([]);
expect(console.error.mock.calls).toEqual([
[
// Reported by jsdom due to the guarded callback:
expect.objectContaining({
detail: expect.objectContaining({
message: 'Boom',
}),
type: 'unhandled exception',
}),
],
[
// Addendum by React:
expect.stringContaining(
Expand Down
Loading

0 comments on commit de66616

Please sign in to comment.