Skip to content

Commit f830fbc

Browse files
committed
fix: stack overflow when effect has recursion
closes #49
1 parent b5e2ad2 commit f830fbc

File tree

2 files changed

+91
-3
lines changed

2 files changed

+91
-3
lines changed

src/util/ast.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,10 @@ export const isImmediateCall = (node) => {
191191
*
192192
* @returns {Scope.Reference[]}
193193
*/
194-
export const getUpstreamRefs = (context, ref) => {
195-
if (!ref.resolved) {
194+
export const getUpstreamRefs = (context, ref, visited = new Set()) => {
195+
if (visited.has(ref)) {
196+
return [];
197+
} else if (!ref.resolved) {
196198
// I think this only happens when:
197199
// 1. Import statement is missing
198200
// 2. ESLint globals are misconfigured
@@ -208,6 +210,10 @@ export const getUpstreamRefs = (context, ref) => {
208210
return [];
209211
}
210212

213+
// TODO: Probably best to track this here but let the downstream `traverse()` handle it.
214+
// Especially if we can simplify/eliminate `getDownstreamRefs()` -> `findDownstreamNodes()` from the path.
215+
visited.add(ref);
216+
211217
const upstreamRefs = ref.resolved.defs
212218
// TODO: https://github.com/NickvanDyke/eslint-plugin-react-you-might-not-need-an-effect/issues/34
213219
// `init` covers for arrow functions; also needs `body` to descend into function declarations
@@ -220,7 +226,7 @@ export const getUpstreamRefs = (context, ref) => {
220226
// May not be necessary if we adapt the check in `isState()`?
221227
.filter((def) => !isUseState(def.node))
222228
.flatMap((def) => getDownstreamRefs(context, def.node.init))
223-
.flatMap((ref) => getUpstreamRefs(context, ref));
229+
.flatMap((ref) => getUpstreamRefs(context, ref, visited));
224230

225231
// Ultimately return only leaf refs
226232
return upstreamRefs.length === 0 ? [ref] : upstreamRefs;

test/real-world.test.js

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,88 @@ describe("recommended rules on real-world code", () => {
332332
};
333333
`,
334334
},
335+
{
336+
// https://github.com/NickvanDyke/eslint-plugin-react-you-might-not-need-an-effect/issues/49
337+
name: "Effect with recursion", // Particularly likely for `no-manage-parent` because it checks *all* refs in the effect
338+
code: js`
339+
function Component() {
340+
useEffect(() => {
341+
const container = ctnDom.current
342+
if (!container) return
343+
344+
// WebGL setup (~30 lines)
345+
const renderer = new Renderer({ alpha: true, premultipliedAlpha: false })
346+
const gl = renderer.gl
347+
const program = new Program(gl, {
348+
vertex: vert,
349+
fragment: frag,
350+
uniforms: {
351+
iTime: { value: 0 },
352+
iResolution: { value: new Vec3(gl.canvas.width, gl.canvas.height, gl.canvas.width / gl.canvas.height) },
353+
hue: { value: hue },
354+
hover: { value: 0 },
355+
rot: { value: 0 },
356+
hoverIntensity: { value: hoverIntensity },
357+
},
358+
})
359+
const mesh = new Mesh(gl, { geometry, program })
360+
361+
// Nested function 4: animation loop with recursion
362+
let rafId
363+
let lastTime = 0
364+
let currentRot = 0
365+
const rotationSpeed = 0.3
366+
367+
const update = (t) => {
368+
rafId = requestAnimationFrame(update) // Recursive call
369+
const dt = (t - lastTime) * 0.001
370+
lastTime = t
371+
const currentTime = t * 0.001
372+
program.uniforms.iTime.value = currentTime
373+
374+
// Color cycle
375+
if (cycleHue) {
376+
const cyclicHue = (hue + currentTime * hueCycleSpeed) % 360
377+
program.uniforms.hue.value = cyclicHue
378+
} else {
379+
program.uniforms.hue.value = hue
380+
}
381+
382+
program.uniforms.hoverIntensity.value = hoverIntensity
383+
384+
const effectiveHover = forceHoverState ? 1 : targetHover
385+
program.uniforms.hover.value += (effectiveHover - program.uniforms.hover.value) * 0.1
386+
387+
if (rotateOnHover && effectiveHover > 0.5) {
388+
currentRot += dt * rotationSpeed
389+
}
390+
program.uniforms.rot.value = currentRot
391+
392+
renderer.render({ scene: mesh })
393+
}
394+
rafId = requestAnimationFrame(update)
395+
396+
// Cleanup function
397+
return () => {
398+
cancelAnimationFrame(rafId)
399+
window.removeEventListener("resize", resize)
400+
container.removeEventListener("mousemove", handleMouseMove)
401+
container.removeEventListener("mouseleave", handleMouseLeave)
402+
container.removeChild(gl.canvas)
403+
gl.getExtension("WEBGL_lose_context")?.loseContext()
404+
}
405+
}, [
406+
hue,
407+
hoverIntensity,
408+
rotateOnHover,
409+
forceHoverState,
410+
cycleHue,
411+
hueCycleSpeed,
412+
size,
413+
])
414+
}
415+
`,
416+
},
335417
].forEach(({ name, code }) => {
336418
it(name, async () => {
337419
const results = await eslint.lintText(code);

0 commit comments

Comments
 (0)