Skip to content

Commit

Permalink
Merge pull request #2358 from preactjs/remove-effect-warnings
Browse files Browse the repository at this point in the history
remove warning for use(Layout)Effect
  • Loading branch information
marvinhagemeister authored Feb 18, 2020
2 parents 4a073c8 + 4a65ac1 commit 0da8005
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 84 deletions.
40 changes: 1 addition & 39 deletions debug/src/debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export function initDebug() {
default:
isValid = false;
}

if (!isValid) {
let componentName = getDisplayName(vnode);
throw new Error(
Expand Down Expand Up @@ -298,45 +299,6 @@ export function initDebug() {
}
});
}

// After paint effects
if (Array.isArray(hooks._pendingEffects)) {
hooks._pendingEffects.forEach(effect => {
if (
!Array.isArray(effect._args) &&
warnedComponents &&
!warnedComponents.useEffect.has(vnode.type)
) {
warnedComponents.useEffect.set(vnode.type, true);
let componentName = getDisplayName(vnode);
console.warn(
'You should provide an array of arguments as the second argument to the "useEffect" hook.\n\n' +
'Not doing so will invoke this effect on every render.\n\n' +
`This effect can be found in the render of ${componentName}.` +
`\n\n${getOwnerStack(vnode)}`
);
}
});
}

// Layout Effects
component._renderCallbacks.forEach(possibleEffect => {
if (
possibleEffect._value &&
!Array.isArray(possibleEffect._args) &&
warnedComponents &&
!warnedComponents.useLayoutEffect.has(vnode.type)
) {
warnedComponents.useLayoutEffect.set(vnode.type, true);
let componentName = getDisplayName(vnode);
console.warn(
'You should provide an array of arguments as the second argument to the "useLayoutEffect" hook.\n\n' +
'Not doing so will invoke this effect on every render.\n\n' +
`This effect can be found in the render of ${componentName}.` +
`\n\n${getOwnerStack(vnode)}`
);
}
});
}

if (oldDiffed) oldDiffed(vnode);
Expand Down
48 changes: 3 additions & 45 deletions debug/test/browser/debug-hooks.test.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import { createElement, render, Component } from 'preact';
import {
useState,
useEffect,
useLayoutEffect,
useMemo,
useCallback
} from 'preact/hooks';
import { useState, useEffect, useMemo, useCallback } from 'preact/hooks';
import 'preact/debug';
import { act } from 'preact/test-utils';
import { setupScratch, teardown } from '../../../test/_util/helpers';
Expand Down Expand Up @@ -91,43 +85,6 @@ describe('debug with hooks', () => {
expect(fn).to.throw(/Hook can only be invoked from render/);
});

it('should warn for argumentless useEffect hooks', () => {
const App = () => {
const [state] = useState('test');
useEffect(() => 'test');
return <p>{state}</p>;
};
render(<App />, scratch);
// Skip first warning which is about missing babel plugin for better
// debug messages
expect(warnings[1]).to.match(/You should provide an array of arguments/);
render(<App />, scratch);
expect(warnings[2]).to.be.undefined;
});

it('should warn for argumentless useLayoutEffect hooks', () => {
const App = () => {
const [state] = useState('test');
useLayoutEffect(() => 'test');
return <p>{state}</p>;
};
render(<App />, scratch);
expect(warnings[0]).to.match(/You should provide an array of arguments/);
render(<App />, scratch);
expect(warnings[1]).to.be.undefined;
});

it('should not warn for argumented effect hooks', () => {
const App = () => {
const [state] = useState('test');
useLayoutEffect(() => 'test', []);
useEffect(() => 'test', [state]);
return <p>{state}</p>;
};
const fn = () => act(() => render(<App />, scratch));
expect(fn).to.not.throw();
});

it('should warn for useMemo/useCallback without arguments', () => {
const App = () => {
const [people] = useState([40, 20, 60, 80]);
Expand All @@ -136,7 +93,8 @@ describe('debug with hooks', () => {
return <p onClick={cb}>{retiredPeople.map(x => x)}</p>;
};
render(<App />, scratch);
expect(warnings.length).to.equal(2);
// One more to show the need for @babel/plugin-transform-react-jsx-source
expect(warnings.length).to.equal(3);
});

it('should warn when useMemo is called with non-array args', () => {
Expand Down

0 comments on commit 0da8005

Please sign in to comment.