fix: forward style and other props to wrapper div#140
fix: forward style and other props to wrapper div#140This-Is-Ram wants to merge 1 commit intotsparticles:mainfrom
Conversation
📝 WalkthroughWalkthroughThe Particles component's prop handling is refactored to destructure specific props from the rest, spreading remaining props onto the container div. The useEffect dependency array is shortened, and the container ref assignment logic is modified, potentially affecting the cleanup phase. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
components/react/lib/Particles.tsx (1)
21-21: Stale closure risk forparticlesLoadedafter narrowing the dependency array.Removing
propsfrom the dependency array is a good change to avoid unnecessary particle re-initialization. However,props.particlesLoadedis invoked inside the effect (line 15) but isn't tracked in the dependency array, creating a stale closure if the callback identity changes between renders. This is likely fine in practice (the callback fires once on load), but it will triggerreact-hooks/exhaustive-depslint warnings.A common pattern to address this without causing re-runs is to store the callback in a ref:
♻️ Suggested improvement using a ref
-import { FC, useEffect } from "react"; +import { FC, useEffect, useRef } from "react"; import type { IParticlesProps } from "./IParticlesProps"; import { tsParticles, type Container } from "@tsparticles/engine"; const Particles: FC<IParticlesProps> = (props) => { const id = props.id ?? "tsparticles"; + const particlesLoadedRef = useRef(props.particlesLoaded); + particlesLoadedRef.current = props.particlesLoaded; useEffect(() => { let container: Container | undefined; tsParticles .load({ id, url: props.url, options: props.options }) .then((c) => { container = c; - props.particlesLoaded?.(c); + particlesLoadedRef.current?.(c); }); return () => { container?.destroy(); }; }, [id, props.url, props.options]);
The style prop was not being forwarded to the wrapper div,
causing it to have no effect.
This change forwards the style and other remaining props
to the wrapper div so they work correctly.
Fixes #119.
Summary by CodeRabbit