Skip to content

Commit

Permalink
Catch exceptions when flushing effects during act
Browse files Browse the repository at this point in the history
Make sure that future state updates and effects continue to run if an
exception occurs during a call to `act`.

This required resolving an issue with hooks that an exception inside
`useEffect` left the effect callback in the effect queue, so it would
run again the next time effects were flushed.
  • Loading branch information
robertknight committed Mar 31, 2020
1 parent cbf41e2 commit 4efcd75
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 11 deletions.
7 changes: 4 additions & 3 deletions hooks/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,11 @@ export function useErrorBoundary(cb) {
function flushAfterPaintEffects() {
afterPaintEffects.some(component => {
if (component._parentDom) {
const effects = component.__hooks._pendingEffects;
component.__hooks._pendingEffects = [];
try {
component.__hooks._pendingEffects.forEach(invokeCleanup);
component.__hooks._pendingEffects.forEach(invokeEffect);
component.__hooks._pendingEffects = [];
effects.forEach(invokeCleanup);
effects.forEach(invokeEffect);
} catch (e) {
options._catchError(e, component._vnode);
return true;
Expand Down
23 changes: 15 additions & 8 deletions test-utils/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export function act(cb) {
// outermost call returns. In the inner call, we just execute the
// callback and return since the infrastructure for flushing has already
// been set up.
//
// If an exception occurs, the outermost `act` will handle cleanup.
const result = cb();
if (isThenable(result)) {
return result.then(() => {
Expand All @@ -50,18 +52,23 @@ export function act(cb) {
options.requestAnimationFrame = fc => (flush = fc);

const finish = () => {
rerender();
while (flush) {
toFlush = flush;
flush = null;

toFlush();
try {
rerender();
while (flush) {
toFlush = flush;
flush = null;

toFlush();
rerender();
}
teardown();
} catch (e) {
if (!err) {
err = e;
}
}

teardown();
options.requestAnimationFrame = previousRequestAnimationFrame;

--actDepth;
};

Expand Down
37 changes: 37 additions & 0 deletions test-utils/test/shared/act.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -438,5 +438,42 @@ describe('act', () => {
expect(effectCount).to.equal(1);
});
});

context('in an effect', () => {
function BrokenEffect() {
useEffect(() => {
throw new Error('BrokenEffect effect');
}, []);
return null;
}

const renderBrokenEffect = () => {
act(() => {
render(<BrokenEffect />, scratch);
});
};

it('should rethrow the exception', () => {
expect(renderBrokenEffect).to.throw('BrokenEffect effect');
});

it('should not affect state updates in future renders', () => {
try {
renderBrokenEffect();
} catch (e) {}

renderWorking();
expect(scratch.textContent).to.equal('1');
});

it('should not affect effects in future renders', () => {
try {
renderBrokenEffect();
} catch (e) {}

renderWorking();
expect(effectCount).to.equal(1);
});
});
});
});

0 comments on commit 4efcd75

Please sign in to comment.