Skip to content

Commit

Permalink
update error message to include useLayoutEffect or useEffect on bad e… (
Browse files Browse the repository at this point in the history 
#22279)

* update error message to include useLayoutEffect or useEffect on bad effect return

* Update packages/react-reconciler/src/ReactFiberCommitWork.new.js

Co-authored-by: Ricky <rickhanlonii@gmail.com>

* use existing import

Co-authored-by: Ricky <rickhanlonii@gmail.com>
  • Loading branch information
@salazarm @rickhanlonii
salazarm and rickhanlonii authored Sep 10, 2021
1 parent cb8a506 commit 0fd195f
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 15 deletions.
18 changes: 14 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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' +
Expand All @@ -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,
);
}
Expand Down
18 changes: 14 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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' +
Expand All @@ -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,
);
}
Expand Down
14 changes: 7 additions & 7 deletions packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
]);

Expand All @@ -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).',
]);
Expand All @@ -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.',
]);
Expand Down Expand Up @@ -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',
]);

Expand All @@ -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).',
]);
Expand All @@ -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
Expand Down

0 comments on commit 0fd195f

Please sign in to comment.