Skip to content

fix(useIntersectionObserver): preserve observer across renders when options are structurally equal - #467

Merged
hyesungoh merged 3 commits into
toss:mainfrom
constantly-dev:fix/intersection-observer-unstable-options
Sep 3, 2026
Merged

fix(useIntersectionObserver): preserve observer across renders when options are structurally equal#467
hyesungoh merged 3 commits into
toss:mainfrom
constantly-dev:fix/intersection-observer-unstable-options

Conversation

@constantly-dev

Copy link
Copy Markdown
Contributor

Overview

useIntersectionObserver recreates its IntersectionObserver on every render where the caller passes an inline options object, even when root/rootMargin/threshold haven't actually changed. This is because options is passed directly into the useMemo/useRefEffect dependency arrays, and an inline object literal gets a new reference every render.

useImpressionRef hits this on every re-render since it always builds a fresh options object internally ({ rootMargin, threshold: areaThreshold }), so this affects any frequently re-rendering component using it (list items, ads, etc.).

Fix

Wrapped options with usePreservedReference and a custom equality function, areIntersectionOptionsEqual. A custom comparator was needed instead of the default JSON.stringify comparison because:

  • root is a DOM node, and JSON.stringify on a DOM node always produces "{}" — so the default comparator would treat any two different root elements as equal, silently keeping the observer on a stale root.
  • threshold can be number[], and an inline array gets a new reference every render — so a naive === comparison would defeat the fix for that case.

root is compared by reference; rootMargin/threshold are compared by value.

Verification

  • Confirmed the two new tests fail on main and pass with this fix.
  • Verified with a few deliberately-broken comparator variants that each new test catches a distinct regression (neither is redundant).
  • yarn test:coverage — 100% maintained.

Checklist

  • Did you write the test code?
  • Have you run yarn run fix to format and lint the code and docs?
  • Have you run yarn run test:coverage to make sure there is no uncovered line?
  • Did you write the JSDoc?

Copilot AI lite review requested due to automatic review settings September 3, 2026 02:50
@changeset-bot

changeset-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 84e4621

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
react-simplikit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is narrowly scoped, SSR-safe, and includes targeted tests validating both the new “no recreate on structural equality” behavior and the intended recreation cases.

Pull request overview

This PR improves useIntersectionObserver performance and behavior stability by preventing unnecessary IntersectionObserver re-creation when callers pass inline options objects that are structurally unchanged (especially important for frequently re-rendering consumers like useImpressionRef).

Changes:

  • Preserve IntersectionObserverInit options across renders using usePreservedReference with a custom comparator (areIntersectionOptionsEqual).
  • Add tests to ensure the observer does recreate when root / rootMargin / threshold change, and does not recreate for value-equal threshold arrays with new references.
  • Add a patch changeset documenting the behavior change.
File summaries
File Description
packages/react-simplikit/src/hooks/useIntersectionObserver/useIntersectionObserver.ts Preserves options by structural equality to avoid re-instantiating the observer on referential-only changes.
packages/react-simplikit/src/hooks/useIntersectionObserver/useIntersectionObserver.spec.ts Adds regression tests covering observer recreation rules for option changes vs value-equal threshold arrays.
.changeset/light-jobs-grin.md Patch notes for the behavior change in useIntersectionObserver.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (339ef2f) to head (84e4621).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #467   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           67        67           
  Lines         2199      2204    +5     
  Branches       709       711    +2     
=========================================
+ Hits          2199      2204    +5     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hyesungoh hyesungoh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! please take a look at comments below 🙏

Comment on lines +67 to +70

function areIntersectionOptionsEqual(a: IntersectionObserverInit, b: IntersectionObserverInit): boolean {
return (
a.root === b.root && a.rootMargin === b.rootMargin && JSON.stringify(a.threshold) === JSON.stringify(b.threshold)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a short comment here explaining why root is compared by reference while threshold goes through JSON.stringify? The reasoning is in the PR description, but it would be nice to have it right next to the code. 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hyesungoh
Thanks for the feedback! Pushed a fix in 84e4621.

Let me know if this reads clearly, or if you'd frame it differently — happy to iterate further. 😀

@hyesungoh hyesungoh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appreciate it! 👍

@hyesungoh
hyesungoh merged commit 92cbb34 into toss:main Sep 3, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants