Skip to content

Commit 98c668c

Browse files
committed
Fix crash where a synchronous effect render unmounts the tree (#5026)
(cherry picked from commit 8cbed5f)
1 parent 4885f1d commit 98c668c

File tree

2 files changed

+48
-16
lines changed

2 files changed

+48
-16
lines changed

hooks/src/index.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -444,13 +444,14 @@ export function useId() {
444444
function flushAfterPaintEffects() {
445445
let component;
446446
while ((component = afterPaintEffects.shift())) {
447-
if (!component._parentDom || !component.__hooks) continue;
447+
const hooks = component.__hooks;
448+
if (!component._parentDom || !hooks) continue;
448449
try {
449-
component.__hooks._pendingEffects.some(invokeCleanup);
450-
component.__hooks._pendingEffects.some(invokeEffect);
451-
component.__hooks._pendingEffects = [];
450+
hooks._pendingEffects.some(invokeCleanup);
451+
hooks._pendingEffects.some(invokeEffect);
452+
hooks._pendingEffects = [];
452453
} catch (e) {
453-
component.__hooks._pendingEffects = [];
454+
hooks._pendingEffects = [];
454455
options._catchError(e, component._vnode);
455456
}
456457
}

hooks/test/browser/useEffect.test.jsx

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1+
import { Component, Fragment, createElement, render } from 'preact';
2+
import { useEffect, useRef, useState } from 'preact/hooks';
13
import { act, teardown as teardownAct } from 'preact/test-utils';
2-
import { createElement, render, Fragment, Component } from 'preact';
3-
import { useEffect, useState, useRef } from 'preact/hooks';
4+
import { vi } from 'vitest';
45
import { setupScratch, teardown } from '../../../test/_util/helpers';
5-
import { useEffectAssertions } from './useEffectAssertions';
66
import { scheduleEffectAssert } from '../_util/useEffectUtil';
7-
import { vi } from 'vitest';
7+
import { useEffectAssertions } from './useEffectAssertions';
88

99
describe('useEffect', () => {
1010
/** @type {HTMLDivElement} */
@@ -84,7 +84,7 @@ describe('useEffect', () => {
8484
});
8585

8686
it('should execute effects in parent if child throws in effect', async () => {
87-
let executionOrder = [];
87+
const executionOrder = [];
8888

8989
const Child = () => {
9090
useEffect(() => {
@@ -268,8 +268,8 @@ describe('useEffect', () => {
268268
scratch.appendChild(global);
269269

270270
const Modal = props => {
271-
let [, setCanProceed] = useState(true);
272-
let ChildProp = props.content;
271+
const [, setCanProceed] = useState(true);
272+
const ChildProp = props.content;
273273

274274
return (
275275
<div>
@@ -425,9 +425,11 @@ describe('useEffect', () => {
425425
}
426426

427427
render() {
428-
return this.state.error
429-
? <h2>Error! {this.state.error}</h2>
430-
: this.props.children;
428+
return this.state.error ? (
429+
<h2>Error! {this.state.error}</h2>
430+
) : (
431+
this.props.children
432+
);
431433
}
432434
}
433435

@@ -634,6 +636,35 @@ describe('useEffect', () => {
634636
expect(calls).to.deep.equal(['doing effecthi']);
635637
});
636638

639+
it('should not crash when effect throws and component is unmounted by render(null) during flush', () => {
640+
// In flushAfterPaintEffects():
641+
// 1. Guard checks component.__hooks — truthy, passes
642+
// 2. invokeEffect runs the effect callback
643+
// 3. The callback calls render(null, scratch) which unmounts the tree
644+
// → options.unmount sets component.__hooks = undefined
645+
// 4. Resetting the hooks array to an empty array would throw an error
646+
let setVal;
647+
648+
function App() {
649+
const [val, _setVal] = useState(0);
650+
setVal = _setVal;
651+
useEffect(() => {
652+
if (val === 1) {
653+
render(null, scratch);
654+
}
655+
}, [val]);
656+
return <div>val: {val}</div>;
657+
}
658+
659+
act(() => {
660+
render(<App />, scratch);
661+
});
662+
663+
act(() => {
664+
setVal(1);
665+
});
666+
});
667+
637668
it('should not rerun when receiving NaN on subsequent renders', () => {
638669
const calls = [];
639670
const Component = ({ value }) => {
@@ -647,7 +678,7 @@ describe('useEffect', () => {
647678
}, [value]);
648679
return <p>{count}</p>;
649680
};
650-
const App = () => <Component value={NaN} />;
681+
const App = () => <Component value={Number.NaN} />;
651682

652683
act(() => {
653684
render(<App />, scratch);

0 commit comments

Comments
 (0)