Skip to content

Commit

Permalink
Allow async blocks in to(Error|Warn)Dev (#25338)
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon authored Dec 1, 2022
1 parent f197ca9 commit 3ba7add
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1777,7 +1777,7 @@ describe('ReactHooksWithNoopRenderer', () => {
}, [props.count]);
return <Text text={'Count: ' + count} />;
}
expect(() =>
expect(() => {
act(() => {
ReactNoop.render(<Counter count={0} />, () =>
Scheduler.unstable_yieldValue('Sync effect'),
Expand All @@ -1787,8 +1787,8 @@ describe('ReactHooksWithNoopRenderer', () => {
'Sync effect',
]);
expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
}),
).toErrorDev('flushSync was called from inside a lifecycle method');
});
}).toErrorDev('flushSync was called from inside a lifecycle method');
expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
});

Expand Down Expand Up @@ -2648,32 +2648,32 @@ describe('ReactHooksWithNoopRenderer', () => {
}

const root1 = ReactNoop.createRoot();
expect(() =>
expect(() => {
act(() => {
root1.render(<App return={17} />);
}),
).toErrorDev([
});
}).toErrorDev([
'Warning: useEffect must not return anything besides a ' +
'function, which is used for clean-up. You returned: 17',
]);

const root2 = ReactNoop.createRoot();
expect(() =>
expect(() => {
act(() => {
root2.render(<App return={null} />);
}),
).toErrorDev([
});
}).toErrorDev([
'Warning: useEffect must not return anything besides a ' +
'function, which is used for clean-up. You returned null. If your ' +
'effect does not require clean up, return undefined (or nothing).',
]);

const root3 = ReactNoop.createRoot();
expect(() =>
expect(() => {
act(() => {
root3.render(<App return={Promise.resolve()} />);
}),
).toErrorDev([
});
}).toErrorDev([
'Warning: useEffect must not return anything besides a ' +
'function, which is used for clean-up.\n\n' +
'It looks like you wrote useEffect(async () => ...) or returned a Promise.',
Expand Down Expand Up @@ -3052,32 +3052,32 @@ describe('ReactHooksWithNoopRenderer', () => {
}

const root1 = ReactNoop.createRoot();
expect(() =>
expect(() => {
act(() => {
root1.render(<App return={17} />);
}),
).toErrorDev([
});
}).toErrorDev([
'Warning: useInsertionEffect must not return anything besides a ' +
'function, which is used for clean-up. You returned: 17',
]);

const root2 = ReactNoop.createRoot();
expect(() =>
expect(() => {
act(() => {
root2.render(<App return={null} />);
}),
).toErrorDev([
});
}).toErrorDev([
'Warning: useInsertionEffect must not return anything besides a ' +
'function, which is used for clean-up. You returned null. If your ' +
'effect does not require clean up, return undefined (or nothing).',
]);

const root3 = ReactNoop.createRoot();
expect(() =>
expect(() => {
act(() => {
root3.render(<App return={Promise.resolve()} />);
}),
).toErrorDev([
});
}).toErrorDev([
'Warning: useInsertionEffect must not return anything besides a ' +
'function, which is used for clean-up.\n\n' +
'It looks like you wrote useInsertionEffect(async () => ...) or returned a Promise.',
Expand All @@ -3104,11 +3104,11 @@ describe('ReactHooksWithNoopRenderer', () => {
}

const root = ReactNoop.createRoot();
expect(() =>
expect(() => {
act(() => {
root.render(<App />);
}),
).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']);
});
}).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']);

expect(() => {
act(() => {
Expand Down Expand Up @@ -3359,32 +3359,32 @@ describe('ReactHooksWithNoopRenderer', () => {
}

const root1 = ReactNoop.createRoot();
expect(() =>
expect(() => {
act(() => {
root1.render(<App return={17} />);
}),
).toErrorDev([
});
}).toErrorDev([
'Warning: useLayoutEffect must not return anything besides a ' +
'function, which is used for clean-up. You returned: 17',
]);

const root2 = ReactNoop.createRoot();
expect(() =>
expect(() => {
act(() => {
root2.render(<App return={null} />);
}),
).toErrorDev([
});
}).toErrorDev([
'Warning: useLayoutEffect must not return anything besides a ' +
'function, which is used for clean-up. You returned null. If your ' +
'effect does not require clean up, return undefined (or nothing).',
]);

const root3 = ReactNoop.createRoot();
expect(() =>
expect(() => {
act(() => {
root3.render(<App return={Promise.resolve()} />);
}),
).toErrorDev([
});
}).toErrorDev([
'Warning: useLayoutEffect must not return anything besides a ' +
'function, which is used for clean-up.\n\n' +
'It looks like you wrote useLayoutEffect(async () => ...) or returned a Promise.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ describe 'ReactCoffeeScriptClass', ->
expect(->
act ->
root.render React.createElement(Foo, foo: 'foo')
return
).toErrorDev 'Foo: getDerivedStateFromProps() is defined as an instance method and will be ignored. Instead, declare it as a static method.'

it 'warns if getDerivedStateFromError is not static', ->
Expand All @@ -144,6 +145,7 @@ describe 'ReactCoffeeScriptClass', ->
expect(->
act ->
root.render React.createElement(Foo, foo: 'foo')
return
).toErrorDev 'Foo: getDerivedStateFromError() is defined as an instance method and will be ignored. Instead, declare it as a static method.'

it 'warns if getSnapshotBeforeUpdate is static', ->
Expand All @@ -155,6 +157,7 @@ describe 'ReactCoffeeScriptClass', ->
expect(->
act ->
root.render React.createElement(Foo, foo: 'foo')
return
).toErrorDev 'Foo: getSnapshotBeforeUpdate() is defined as a static method and will be ignored. Instead, declare it as an instance method.'

it 'warns if state not initialized before static getDerivedStateFromProps', ->
Expand All @@ -171,6 +174,7 @@ describe 'ReactCoffeeScriptClass', ->
expect(->
act ->
root.render React.createElement(Foo, foo: 'foo')
return
).toErrorDev (
'`Foo` uses `getDerivedStateFromProps` but its initial state is ' +
'undefined. This is not recommended. Instead, define the initial state by ' +
Expand Down
16 changes: 12 additions & 4 deletions packages/react/src/__tests__/ReactES6Class-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,9 @@ describe('ReactES6Class', () => {
return <div />;
}
}
expect(() => act(() => root.render(<Foo foo="foo" />))).toErrorDev(
expect(() => {
act(() => root.render(<Foo foo="foo" />));
}).toErrorDev(
'Foo: getDerivedStateFromProps() is defined as an instance method ' +
'and will be ignored. Instead, declare it as a static method.',
);
Expand All @@ -164,7 +166,9 @@ describe('ReactES6Class', () => {
return <div />;
}
}
expect(() => act(() => root.render(<Foo foo="foo" />))).toErrorDev(
expect(() => {
act(() => root.render(<Foo foo="foo" />));
}).toErrorDev(
'Foo: getDerivedStateFromError() is defined as an instance method ' +
'and will be ignored. Instead, declare it as a static method.',
);
Expand All @@ -177,7 +181,9 @@ describe('ReactES6Class', () => {
return <div />;
}
}
expect(() => act(() => root.render(<Foo foo="foo" />))).toErrorDev(
expect(() => {
act(() => root.render(<Foo foo="foo" />));
}).toErrorDev(
'Foo: getSnapshotBeforeUpdate() is defined as a static method ' +
'and will be ignored. Instead, declare it as an instance method.',
);
Expand All @@ -195,7 +201,9 @@ describe('ReactES6Class', () => {
return <div className={`${this.state.foo} ${this.state.bar}`} />;
}
}
expect(() => act(() => root.render(<Foo foo="foo" />))).toErrorDev(
expect(() => {
act(() => root.render(<Foo foo="foo" />));
}).toErrorDev(
'`Foo` uses `getDerivedStateFromProps` but its initial state is ' +
'undefined. This is not recommended. Instead, define the initial state by ' +
'assigning an object to `this.state` in the constructor of `Foo`. ' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ let useSyncExternalStoreWithSelector;
let React;
let ReactDOM;
let ReactDOMClient;
let ReactFeatureFlags;
let Scheduler;
let act;
let useState;
Expand Down Expand Up @@ -48,6 +49,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
React = require('react');
ReactDOM = require('react-dom');
ReactDOMClient = require('react-dom/client');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
Scheduler = require('scheduler');
useState = React.useState;
useEffect = React.useEffect;
Expand Down Expand Up @@ -882,8 +884,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {

describe('selector and isEqual error handling in extra', () => {
let ErrorBoundary;
beforeAll(() => {
spyOnDev(console, 'warn');
beforeEach(() => {
ErrorBoundary = class extends React.Component {
state = {error: null};
static getDerivedStateFromError(error) {
Expand Down Expand Up @@ -929,9 +930,15 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {

expect(container.textContent).toEqual('A');

await act(() => {
store.set({});
});
await expect(async () => {
await act(async () => {
store.set({});
});
}).toWarnDev(
ReactFeatureFlags.enableUseRefAccessWarning
? ['Warning: App: Unsafe read of a mutable value during render.']
: [],
);
expect(container.textContent).toEqual('Malformed state');
});

Expand Down Expand Up @@ -968,9 +975,15 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {

expect(container.textContent).toEqual('A');

await act(() => {
store.set({});
});
await expect(async () => {
await act(() => {
store.set({});
});
}).toWarnDev(
ReactFeatureFlags.enableUseRefAccessWarning
? ['Warning: App: Unsafe read of a mutable value during render.']
: [],
);
expect(container.textContent).toEqual('Malformed state');
});
});
Expand Down
50 changes: 43 additions & 7 deletions scripts/jest/matchers/toWarnDev.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,7 @@ const createMatcherFor = (consoleMethod, matcherName) =>
// Avoid using Jest's built-in spy since it can't be removed.
console[consoleMethod] = consoleSpy;

try {
callback();
} catch (error) {
caughtError = error;
} finally {
const onFinally = () => {
// Restore the unspied method so that unexpected errors fail tests.
console[consoleMethod] = originalMethod;

Expand Down Expand Up @@ -259,12 +255,52 @@ const createMatcherFor = (consoleMethod, matcherName) =>
}

return {pass: true};
};

let returnPromise = null;
try {
const result = callback();

if (
typeof result === 'object' &&
result !== null &&
typeof result.then === 'function'
) {
// `act` returns a thenable that can't be chained.
// Once `act(async () => {}).then(() => {}).then(() => {})` works
// we can just return `result.then(onFinally, error => ...)`
returnPromise = new Promise((resolve, reject) => {
result.then(
() => {
resolve(onFinally());
},
error => {
caughtError = error;
return resolve(onFinally());
}
);
});
}
} catch (error) {
caughtError = error;
} finally {
return returnPromise === null ? onFinally() : returnPromise;
}
} else {
// Any uncaught errors or warnings should fail tests in production mode.
callback();
const result = callback();

return {pass: true};
if (
typeof result === 'object' &&
result !== null &&
typeof result.then === 'function'
) {
return result.then(() => {
return {pass: true};
});
} else {
return {pass: true};
}
}
};

Expand Down

0 comments on commit 3ba7add

Please sign in to comment.