Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Compat] Throw an error for too many repeated function component rerenders #3965

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 16 additions & 2 deletions compat/src/render.js
Expand Up @@ -261,14 +261,28 @@ options.vnode = vnode => {
if (oldVNodeHook) oldVNodeHook(vnode);
};

// Only needed for react-relay
let renderCount = 0;
let currentComponent;
const oldBeforeRender = options._render;
options._render = function (vnode) {
if (oldBeforeRender) {
oldBeforeRender(vnode);
}
currentComponent = vnode._component;

const nextComponent = vnode._component;
if (nextComponent === currentComponent) {
renderCount++;
} else {
renderCount = 1;
}

if (renderCount >= 25) {
throw new Error(
`Too many re-renders. Preact/compat limits the number of renders to prevent an infinite loop.`
);
}

currentComponent = nextComponent;
};

const oldDiffed = options.diffed;
Expand Down
35 changes: 34 additions & 1 deletion compat/test/browser/render.test.js
Expand Up @@ -3,7 +3,9 @@ import React, {
render,
Component,
hydrate,
createContext
createContext,
useState,
Fragment
} from 'preact/compat';
import { setupRerender, act } from 'preact/test-utils';
import {
Expand Down Expand Up @@ -512,4 +514,35 @@ describe('compat render', () => {

expect(scratch.textContent).to.equal('foo');
});

it('throws an error if a component rerenders too many times', () => {
let rerenderCount = 0;
function TestComponent({ loop = false }) {
const [count, setCount] = useState(0);
if (loop) {
setCount(count + 1);
}

if (count > 30) {
expect.fail(
'Repeated rerenders did not cause the expected error. This test is failing.'
);
}

rerenderCount += 1;
return <div />;
}

expect(() => {
render(
<Fragment>
<TestComponent />
<TestComponent loop />
</Fragment>,
scratch
);
}).to.throw(/Too many re-renders/);
// 1 for first TestComponent + 24 for second TestComponent
expect(rerenderCount).to.equal(25);
});
});