Skip to content

update error message to include useLayoutEffect or useEffect on bad e… #22279

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

Merged
merged 3 commits into from
Sep 10, 2021
Merged
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
18 changes: 14 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
@@ -507,7 +507,7 @@ function commitHookEffectListUnmount(
}
}

function commitHookEffectListMount(tag: number, finishedWork: Fiber) {
function commitHookEffectListMount(tag: HookFlags, finishedWork: Fiber) {
const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any);
const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null;
if (lastEffect !== null) {
@@ -522,17 +522,26 @@ function commitHookEffectListMount(tag: number, finishedWork: Fiber) {
if (__DEV__) {
const destroy = effect.destroy;
if (destroy !== undefined && typeof destroy !== 'function') {
let hookName;
if ((effect.tag & HookLayout) !== NoFlags) {
hookName = 'useLayoutEffect';
} else {
hookName = 'useEffect';
}
let addendum;
if (destroy === null) {
addendum =
' You returned null. If your effect does not require clean ' +
'up, return undefined (or nothing).';
} else if (typeof destroy.then === 'function') {
addendum =
'\n\nIt looks like you wrote useEffect(async () => ...) or returned a Promise. ' +
'\n\nIt looks like you wrote ' +
hookName +
'(async () => ...) or returned a Promise. ' +
'Instead, write the async function inside your effect ' +
'and call it immediately:\n\n' +
'useEffect(() => {\n' +
hookName +
'(() => {\n' +
' async function fetchData() {\n' +
' // You can await here\n' +
' const response = await MyAPI.getData(someId);\n' +
@@ -545,8 +554,9 @@ function commitHookEffectListMount(tag: number, finishedWork: Fiber) {
addendum = ' You returned: ' + destroy;
}
console.error(
'An effect function must not return anything besides a function, ' +
'%s must not return anything besides a function, ' +
'which is used for clean-up.%s',
hookName,
addendum,
);
}
18 changes: 14 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
Original file line number Diff line number Diff line change
@@ -507,7 +507,7 @@ function commitHookEffectListUnmount(
}
}

function commitHookEffectListMount(tag: number, finishedWork: Fiber) {
function commitHookEffectListMount(tag: HookFlags, finishedWork: Fiber) {
const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any);
const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null;
if (lastEffect !== null) {
@@ -522,17 +522,26 @@ function commitHookEffectListMount(tag: number, finishedWork: Fiber) {
if (__DEV__) {
const destroy = effect.destroy;
if (destroy !== undefined && typeof destroy !== 'function') {
let hookName;
if ((effect.tag & HookLayout) !== NoFlags) {
hookName = 'useLayoutEffect';
} else {
hookName = 'useEffect';
}
let addendum;
if (destroy === null) {
addendum =
' You returned null. If your effect does not require clean ' +
'up, return undefined (or nothing).';
} else if (typeof destroy.then === 'function') {
addendum =
'\n\nIt looks like you wrote useEffect(async () => ...) or returned a Promise. ' +
'\n\nIt looks like you wrote ' +
hookName +
'(async () => ...) or returned a Promise. ' +
'Instead, write the async function inside your effect ' +
'and call it immediately:\n\n' +
'useEffect(() => {\n' +
hookName +
'(() => {\n' +
' async function fetchData() {\n' +
' // You can await here\n' +
' const response = await MyAPI.getData(someId);\n' +
@@ -545,8 +554,9 @@ function commitHookEffectListMount(tag: number, finishedWork: Fiber) {
addendum = ' You returned: ' + destroy;
}
console.error(
'An effect function must not return anything besides a function, ' +
'%s must not return anything besides a function, ' +
'which is used for clean-up.%s',
hookName,
addendum,
);
}
Original file line number Diff line number Diff line change
@@ -2647,7 +2647,7 @@ describe('ReactHooksWithNoopRenderer', () => {
root1.render(<App return={17} />);
}),
).toErrorDev([
'Warning: An effect function must not return anything besides a ' +
'Warning: useEffect must not return anything besides a ' +
'function, which is used for clean-up. You returned: 17',
]);

@@ -2657,7 +2657,7 @@ describe('ReactHooksWithNoopRenderer', () => {
root2.render(<App return={null} />);
}),
).toErrorDev([
'Warning: An effect function must not return anything besides a ' +
'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).',
]);
@@ -2668,7 +2668,7 @@ describe('ReactHooksWithNoopRenderer', () => {
root3.render(<App return={Promise.resolve()} />);
}),
).toErrorDev([
'Warning: An effect function must not return anything besides a ' +
'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.',
]);
@@ -2873,7 +2873,7 @@ describe('ReactHooksWithNoopRenderer', () => {
root1.render(<App return={17} />);
}),
).toErrorDev([
'Warning: An effect function must not return anything besides a ' +
'Warning: useLayoutEffect must not return anything besides a ' +
'function, which is used for clean-up. You returned: 17',
]);

@@ -2883,7 +2883,7 @@ describe('ReactHooksWithNoopRenderer', () => {
root2.render(<App return={null} />);
}),
).toErrorDev([
'Warning: An effect function must not return anything besides a ' +
'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).',
]);
@@ -2894,9 +2894,9 @@ describe('ReactHooksWithNoopRenderer', () => {
root3.render(<App return={Promise.resolve()} />);
}),
).toErrorDev([
'Warning: An effect function must not return anything besides a ' +
'Warning: useLayoutEffect 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.',
'It looks like you wrote useLayoutEffect(async () => ...) or returned a Promise.',
]);

// Error on unmount because React assumes the value is a function