fix: fix fake disappear event #1539
Conversation
🦋 Changeset detectedLatest commit: 8e7d5e4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
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 |
📝 WalkthroughWalkthroughIntroduces a fix in createExposureMonitor to dispatch disexposure when a previously exposed element stops intersecting, adds a new Playwright test and React test page validating no fake disappear, and includes a changeset declaring a patch release for @lynx-js/web-core. No public APIs change. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #1539 will not alter performanceComparing Summary
|
React Example#4363 Bundle Size — 237.04KiB (0%).8e7d5e4(current) vs 31f3175 main#4360(baseline) Bundle metrics
|
| Current #4363 |
Baseline #4360 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
158 |
158 |
| Duplicate Modules |
64 |
64 |
| Duplicate Code |
45.86% |
45.86% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #4363 |
Baseline #4360 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.28KiB |
91.28KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-exposure-fake... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#4356 Bundle Size — 366.68KiB (~+0.01%).8e7d5e4(current) vs 31f3175 main#4353(baseline) Bundle metrics
|
| Current #4356 |
Baseline #4353 |
|
|---|---|---|
 Initial JS |
143.49KiB(~+0.01%) |
143.48KiB |
| Initial CSS |
31.84KiB |
31.84KiB |
 Cache Invalidation |
39.13% |
0% |
| Chunks |
7 |
7 |
| Assets |
7 |
7 |
| Modules |
212(+0.95%) |
210 |
| Duplicate Modules |
17 |
17 |
| Duplicate Code |
3.89% |
3.89% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #4356 |
Baseline #4353 |
|
|---|---|---|
| JS |
234.68KiB (~+0.01%) |
234.66KiB |
| Other |
100.16KiB |
100.16KiB |
| CSS |
31.84KiB |
31.84KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-exposure-fake... Project dashboard
Generated by RelativeCI Documentation Report issue
PupilTong
force-pushed
the
p/hw/fix-exposure-fake-disappear
branch
from
695b05c to
0fb51d8
Compare
PupilTong
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the exposure monitoring system where fake uidisappear events were being triggered when elements became visible. The fix ensures that disappear events are only sent when elements actually transition from visible to not visible.
Key Changes
- Fixed logic in the Intersection Observer to prevent fake disappear events
- Added a test case to verify the fix works correctly
- Updated changeset documentation
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
packages/web-platform/web-core/src/uiThread/crossThreadHandlers/createExposureMonitor.ts |
Fixed the condition to only send disappear events when elements actually disappear |
packages/web-platform/web-tests/tests/react/api-exposure-no-fake-disappear/index.jsx |
Added test component to verify the fix |
packages/web-platform/web-tests/tests/react.spec.ts |
Added test case for the fix |
.changeset/modern-wombats-float.md |
Added changeset documentation |
packages/web-platform/web-tests/tests/react/api-exposure-no-fake-disappear/index.jsx
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.changeset/modern-wombats-float.md (1)
5-5: Polish the changeset description for clarity.The current phrasing is terse and slightly ungrammatical. Consider a clearer, release-note-friendly message.
Apply this diff:
-fix: fake uidisappear event +fix: prevent false uidisappear/disexposure eventspackages/web-platform/web-tests/tests/react.spec.ts (1)
971-985: Solid regression test; consider a slightly longer stabilization wait to reduce flakiness.The assertions correctly validate that neither uiappear nor uidisappear was spuriously fired for #target. To make the test more resilient across CI environments where IO/paint scheduling varies, bump the wait a bit.
- await wait(300); + await wait(500);packages/web-platform/web-core/src/uiThread/crossThreadHandlers/createExposureMonitor.ts (1)
56-68: Optional: Base exposure state on intersectionRatio vs. isIntersecting to honor exposure-area thresholds.Using isIntersecting alone does not reflect configured exposure-area thresholds. When threshold > 0, entries fire on threshold crossings even while isIntersecting stays true; in that case, we won’t send disexposure when the ratio drops below threshold but remains > 0.
A simple refinement is to track the per-target threshold and compare intersectionRatio to that threshold.
Apply this diff to the callback:
- const intersectionObserverCallback = ( - entries: IntersectionObserverEntry[], - ) => { - entries.forEach(({ target, isIntersecting }) => { - if (isIntersecting && !exposedElements.has(target)) { - sendExposureEvent(target, true, target.getAttribute('exposure-id')); - exposedElements.add(target); - } else if (!isIntersecting && exposedElements.has(target)) { - sendExposureEvent(target, false, target.getAttribute('exposure-id')); - exposedElements.delete(target); - } - }); - }; + const intersectionObserverCallback = ( + entries: IntersectionObserverEntry[], + ) => { + entries.forEach(({ target, intersectionRatio }) => { + const threshold = thresholds.get(target) ?? 0; + const above = intersectionRatio >= threshold; + if (above && !exposedElements.has(target)) { + sendExposureEvent(target, true, target.getAttribute('exposure-id')); + exposedElements.add(target); + } else if (!above && exposedElements.has(target)) { + sendExposureEvent(target, false, target.getAttribute('exposure-id')); + exposedElements.delete(target); + } + }); + };And add the following supportive changes elsewhere:
// near other WeakMaps const thresholds = new WeakMap<Element, number>();// inside startIntersectionObserver, after computing `threshold` thresholds.set(target, threshold);This preserves current behavior for threshold=0 and makes exposure/disexposure align with the configured exposure-area.
packages/web-platform/web-tests/tests/react/api-exposure-no-fake-disappear/index.jsx (1)
31-36: Naming nit: variable suggests only “disappear” but is set on both appear and disappear.For readability, consider renaming uidisappearTriggered to something like exposureEventTriggered (or anyExposureTriggered), since both binduidisappear and binduiappear set it to true.
- const [uidisappearTriggered, setUidisappearTriggered] = useState(false); + const [exposureEventTriggered, setExposureEventTriggered] = useState(false); ... - background: uidisappearTriggered ? 'red' : 'green', + background: exposureEventTriggered ? 'red' : 'green', ... - binduidisappear={() => { - setUidisappearTriggered(true); + binduidisappear={() => { + setExposureEventTriggered(true); }} - binduiappear={() => { - setUidisappearTriggered(true); + binduiappear={() => { + setExposureEventTriggered(true); }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.changeset/modern-wombats-float.md(1 hunks)packages/web-platform/web-core/src/uiThread/crossThreadHandlers/createExposureMonitor.ts(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/api-exposure-no-fake-disappear/index.jsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/modern-wombats-float.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/modern-wombats-float.md
🪛 LanguageTool
.changeset/modern-wombats-float.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...: patch --- fix: fake uidisappear event
(QB_NEW_EN)
🔇 Additional comments (2)
.changeset/modern-wombats-float.md (1)
2-3: Changeset scope looks correct.Targeting a patch for @lynx-js/web-core aligns with the bugfix nature and matches the code changes under src/. No public API change indicated.
packages/web-platform/web-core/src/uiThread/crossThreadHandlers/createExposureMonitor.ts (1)
63-66: Correctly dispatch disexposure when an exposed element stops intersecting.This fixes the false disappear by ensuring disexposure/uidisappear only fires for elements that were actually exposed. The set bookkeeping is sound.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/[email protected] ### Patch Changes - fix `withInitDataInState` got wrong state in 2nd or more times `defaultDataProcessor`, now it will keep its own state. ([#1478](#1478)) - change `__CreateElement('raw-text')` to `__CreateRawText('')` to avoid `setNativeProps` not working ([#1570](#1570)) - Fix wrong render result when using expression as `key`. ([#1541](#1541)) See [#1371](#1371) for more details. - fix: `Cannot read properties of undefined` error when using `Suspense` ([#1569](#1569)) - Add `animate` API in Main Thread Script(MTS), so you can now control a CSS animation imperatively ([#1534](#1534)) ```ts import type { MainThread } from "@lynx-js/types"; function startAnimation(ele: MainThread.Element) { "main thread"; const animation = ele.animate([{ opacity: 0 }, { opacity: 1 }], { duration: 3000, }); // Can also be paused // animation.pause() } ``` ## @lynx-js/[email protected] ### Patch Changes - Support caching Lynx native events when chunk splitting is enabled. ([#1370](#1370)) When `performance.chunkSplit.strategy` is not `all-in-one`, Lynx native events are cached until the BTS chunk is fully loaded and are replayed when that chunk is ready. The `firstScreenSyncTiming` flag will no longer change to `jsReady` anymore. - Support exporting `Promise` and function in `lynx.config.ts`. ([#1590](#1590)) - Fix missing `publicPath` using when `rspeedy dev --mode production`. ([#1310](#1310)) - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## [email protected] ### Patch Changes - Add `@lynx-js/preact-devtools` by default. ([#1593](#1593)) ## @lynx-js/[email protected] ### Patch Changes - Bump @clack/prompts to v1.0.0-alpha.4 ([#1559](#1559)) ## @lynx-js/[email protected] ### Patch Changes - Support using multiple times in different environments. ([#1498](#1498)) - Support caching Lynx native events when chunk splitting is enabled. ([#1370](#1370)) When `performance.chunkSplit.strategy` is not `all-in-one`, Lynx native events are cached until the BTS chunk is fully loaded and are replayed when that chunk is ready. The `firstScreenSyncTiming` flag will no longer change to `jsReady` anymore. - Updated dependencies \[[`f0d483c`](f0d483c), [`e4d116b`](e4d116b), [`d33c1d2`](d33c1d2)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Support using multiple times in different environments. ([#1498](#1498)) - Alias `@lynx-js/preact-devtools` to `false` to reduce an import of empty webpack module. ([#1593](#1593)) ## @lynx-js/[email protected] ### Patch Changes - Support `lynx.createSelectorQuery().select()` and `setNativeProps` API ([#1570](#1570)) ## @lynx-js/[email protected] ### Patch Changes - fix: globalThis is never accessible in MTS ([#1531](#1531)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: fake uidisappear event ([#1539](#1539)) - Updated dependencies \[[`70863fb`](70863fb)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: globalThis is never accessible in MTS ([#1531](#1531)) - Updated dependencies \[[`70863fb`](70863fb)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: globalThis is never accessible in MTS ([#1531](#1531)) - Updated dependencies \[[`70863fb`](70863fb)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Add new `LynxCacheEventsPlugin`, which will cache Lynx native events until the BTS chunk is fully loaded, and replay them when the BTS chunk is ready. ([#1370](#1370)) - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Always inline the background script that contains rspack runtime module. ([#1582](#1582)) - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Add `lynxCacheEventsSetupList` and `lynxCacheEvents` to RuntimeGlobals. It will be used to cache Lynx native events until the BTS chunk is fully loaded, and replay them when the BTS chunk is ready. ([#1370](#1370)) ## [email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
Summary by CodeRabbit
Bug Fixes
Tests
Chores
Checklist