feat: no-leaked-intersection-observer, closes #1841#1868
Conversation
|
@Netail is attempting to deploy a commit to the Rel1cx's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Technically, you could make a single no-leaked-observer rule, as they are pretty much the same. Then you could also cover MutationObserver |
Rel1cx
left a comment
There was a problem hiding this comment.
The "observe-once" pattern is high-frequency usage specific to IO, no-leaked-intersection-observer.mdx should add a section dedicated to examples, demonstrating the recommended approach (adding disconnect as a fallback in the callback):
useEffect(() => {
const observer = new IntersectionObserver(([entry], obs) => {
if (entry.isIntersecting) {
setVisible(true);
obs.disconnect(); // observe-once
}
});
observer.observe(ref.current);
return () => observer.disconnect(); // still need a fallback: might be unmounted before the callback runs
}, []);
Thanks for the suggestion, but we intentionally keep these as separate rules. While the implementations look nearly identical today, each observer type has its own set of legitimate usage patterns that the rule needs to special-case over time:
Merging them into a single The code duplication is deliberate for the same reason: these rules are expected to diverge as observer-specific handling lands, so copy-then-diverge gives us that freedom without premature abstraction. For this PR specifically, that's also why I'd like to see the IO-specific patterns covered explicitly: a test case for the observe-once callback-parameter pattern, and a docs example showing the recommended way to write it: useEffect(() => {
const observer = new IntersectionObserver(([entry], obs) => {
if (entry.isIntersecting) {
setVisible(true);
obs.disconnect(); // observe-once
}
});
observer.observe(ref.current);
return () => observer.disconnect(); // still need a fallback: might be unmounted before the callback runs
}, []);This example would be meaningless in any other observer's docs, but without it here, users writing observe-once code will likely report the rule's warning as a false positive. That's exactly the kind of per-observer content that justifies keeping the rules separate. |
Signed-off-by: REL1CX <solarflarex@qq.com>
Signed-off-by: REL1CX <solarflarex@qq.com>
What kind of change does this PR introduce?
Check at least one. If you are introducing a new binding, you must reference an issue where this binding has been proposed, discussed and approved by the maintainers.
Does this PR introduce a breaking change?
If yes, please describe the impact and migration path for existing applications in an attached issue.
Checklist
fix: remove a typo, closes #___, #___)Other information