chore(release): v9.14.0 RC - #13822
Conversation
🎉 Beta Versions Generated (commit:
|
Assisted-by: Claude Sonnet 5 <[email protected]>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
* fix: rail tracking * fix: items impression tracking * remove unnecessary test * track all live sections after pull to refresh when in view * chore * chore: edit the way the nwfy view more cta is rendered * chore: revert new santinel placement * chore: tests * chore: revert chore: edit the way the nwfy view more cta is rendered --------- Co-authored-by: dariakoko <[email protected]>
| useEffect(() => { | ||
| if (hasPendingRailViewed && isRailInViewport) { | ||
| tracking.viewedSection(contextModule, index) | ||
| setHasPendingRailViewed(false) | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [hasPendingRailViewed, isRailInViewport]) |
There was a problem hiding this comment.
Two things about this effect:
1. Duplicate railViewed on first scroll-into-view. HomeViewSectionSentinel (rendered at line 216) also fires viewedSection the first time the section becomes visible — its only guard is trackedSections (HomeViewSectionSentinel.tsx:27), which is per-contextModule, not per-refresh. Scenario:
- Cold home view, the recommended/NWFY rail is below the fold → sentinel hasn't fired,
viewableSectionsdoesn't include the section. - User navigates away and back →
useFocusEffectinHomeView.tsx:172bumpsliveRefetchKey→hasPendingRailViewed = true. - User scrolls the rail into view → the outer FlashList sets
viewableSectionsand the sentinel flips visible.
Both paths now call tracking.viewedSection(contextModule, index) → two railViewed events for the same rail in the same moment.
2. railViewed fires before the refreshed data lands. setHasPendingRailViewed(true) runs synchronously on the bump, so when the rail is already on screen this effect fires in the same commit — before fetchQuery resolves, and it fires even if the fetch errors. The old code fired from complete. If that's the intended behaviour, worth saying so in the comment, since "re-fire railViewed after a live refresh" reads as "after the refresh landed".
| // We are using this key to force a re-render to track item impressions again | ||
| key={trackingKey} |
There was a problem hiding this comment.
The comment says "force a re-render", but a changing key forces a full unmount + remount of HomeViewSectionArtworksGrid, which throws away its local state. That resets hasGridLaidOut to false (HomeViewSectionArtworksGrid.tsx:34), so the "View More" button unmounts and only comes back after the async measureInWindow callback lands (HomeViewSectionArtworksGrid.tsx:40-46). Every live refresh of a grid section flashes that CTA out and back in, and remounts all ArtworkGridItems with it.
The useLayoutEffect already re-measures on [artworks], so the remount isn't needed for layout — it's only there to clear the grid's own trackedGridItems ref (line 38), which the parent's resetTracking() doesn't touch. Clearing that set directly is cheaper and avoids the flash, e.g. pass trackingKey down and:
useEffect(() => {
trackedGridItems.clear()
}, [trackingKey])
| const resetTracking = useCallback(() => { | ||
| trackedItems.clear() | ||
| }, [trackedItems]) | ||
| setKey(key + 1) | ||
| }, [trackedItems, key]) |
There was a problem hiding this comment.
setKey(key + 1) reads key from the closure, and resetTracking is invoked from a long-lived async callback (fetchQuery(...).subscribe({ complete }) in HomeViewSectionArtworks.tsx:95) whose enclosing effect deps are just [liveRefetchKey]. It happens to work today because the effect re-runs on every bump, but a functional update removes the staleness question and stops resetTracking's identity from churning on every reset:
| const resetTracking = useCallback(() => { | |
| trackedItems.clear() | |
| }, [trackedItems]) | |
| setKey(key + 1) | |
| }, [trackedItems, key]) | |
| const resetTracking = useCallback(() => { | |
| trackedItems.clear() | |
| setKey((prev) => prev + 1) | |
| }, [trackedItems]) |
| } | ||
| }, | ||
| [enableItemsViewsTracking, isInViewport, contextScreenOwnerType, contextModule] | ||
| [enableItemsViewsTracking, isInViewport, contextScreenOwnerType, contextModule, key] |
There was a problem hiding this comment.
key is added to the useAnimatedReaction deps (production path) but not to the __TEST__ effect at lines 89-100. Nothing in that dep list changes when key bumps — trackItems deps (lines 57-65) don't include key either — so in tests a resetTracking() never re-triggers trackItems on its own.
That means the behaviour this dep buys is untested: HomeViewSectionArtworks.tests.tsx:512 ("re-enables itemViewed tracking after the live refresh completes") passes only because the test manually re-invokes onViewableItemsChanged after the refresh. The real case for the rail path — rail stays exactly in place after returning to home, so viewable items never change — is what the key dep is for and it isn't covered. Adding key to the test effect's deps would make the two paths behave the same and let a test assert it.
Code ReviewSummaryRelease-candidate branch for 9.14.0 carrying three cherry-picks on top of the RC open commit: floor fractional seconds in the countdown ticker (#13830), a copy change on the edit-profile message (#13832), and a rework of home-view live-rail tracking (#13823). The ticker and copy changes are self-contained; all of my comments are on the tracking rework. Issues Found🟡 Duplicate
Both call 🟡 The comment says "force a re-render", but a changing The 🟡
🟡 The
🟢 Use a functional state update —
Areas ReviewedTicker (#13830) — Correct. Copy change (#13832) — No concerns. Performance — Covered above: the grid remount is the only regression I found. Security — Nothing relevant. Questions for Author
Note on this PRThis is the RC tracking PR ("do not merge"), so each cherry-pick was reviewed on its own PR — the screenshot/recording and LLM-disclosure conventions apply there, not here. #13830's commit does carry |
🎉 Beta Versions Generated (commit:
|
4 participants
Release Candidate 9.14.0
This branch was automatically created as the release candidate for version 9.14.0.
Changelog
Cross-platform user-facing changes
iOS user-facing changes
Dev changes
context_module/data_inputtracking gaps in onboarding and Infinite Discovery — iskounen (fix: correct context_module and data_input for onboarding analytics events #13750)initialsfield onOnboardingFollowedArtist— iskounen (fix(DI-358): show real followed artists in the post-follow bottom sheet #13756)ArtistSaveOnboardingBottomSheettoFollowArtistsOnboardingCompletionBottomSheet— iskounen (refactor: rename ArtistSaveOnboardingBottomSheet to FollowArtistsOnboardingCompletionBottomSheet #13776)react-native-reanimated4.2.1 → 4.3.2 andreact-native-worklets0.7.4 → 0.8.3. — gkartalis (fix(PHIRE-3272): bump react-native-reanimated to 4.3.2 #13781)<Theme theme="v3light">instead of theme-adaptive color tokens — iskounen (fix(DI-499): pin new onboarding intro sequence to light theme #13818)#nochangelog