Skip to content

Commit

Permalink
refactor: Warn on NaN in dep arrays instead of throwing (#4527)
Browse files Browse the repository at this point in the history
* refactor: Warn on NaN in dep arrays instead of throwing

* ci: Fix reference to workflow
  • Loading branch information
rschristian authored Oct 12, 2024
1 parent d54bd31 commit 675bcad
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 9 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
uses: andrewiggins/download-base-artifact@v3
with:
artifact: npm-package
workflow: build-test.yml
workflow: ci.yml
required: true
- run: mv preact.tgz preact-main.tgz
- name: Upload locally build & base preact package
Expand Down
2 changes: 1 addition & 1 deletion debug/src/debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ export function initDebug() {
const arg = hook._args[j];
if (isNaN(arg)) {
const componentName = getDisplayName(vnode);
throw new Error(
console.warn(
`Invalid argument passed to hook. Hooks should not be called with NaN in the dependency array. Hook index ${i} in component ${componentName} was called with NaN.`
);
}
Expand Down
21 changes: 14 additions & 7 deletions debug/test/browser/validateHookArgs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,21 @@ describe('Hook argument validation', () => {
};

it(`should error if ${name} is mounted with NaN as an argument`, async () => {
expect(() =>
render(<TestComponent initialValue={NaN} />, scratch)
).to.throw(/Hooks should not be called with NaN in the dependency array/);
render(<TestComponent initialValue={NaN} />, scratch);
expect(console.warn).to.be.calledOnce;
expect(console.warn.args[0]).to.match(
/Hooks should not be called with NaN in the dependency array/
);
});

it(`should error if ${name} is updated with NaN as an argument`, async () => {
render(<TestComponent initialValue={0} />, scratch);

expect(() => {
scratch.querySelector('button').click();
rerender();
}).to.throw(
scratch.querySelector('button').click();
rerender();

expect(console.warn).to.be.calledOnce;
expect(console.warn.args[0]).to.match(
/Hooks should not be called with NaN in the dependency array/
);
});
Expand All @@ -52,14 +55,18 @@ describe('Hook argument validation', () => {
let scratch;
/** @type {() => void} */
let rerender;
let warnings = [];

beforeEach(() => {
scratch = setupScratch();
rerender = setupRerender();
warnings = [];
sinon.stub(console, 'warn').callsFake(w => warnings.push(w));
});

afterEach(() => {
teardown(scratch);
console.warn.restore();
});

validateHook('useEffect', arg => useEffect(() => {}, [arg]));
Expand Down

0 comments on commit 675bcad

Please sign in to comment.