fix: SwipeDeck registry lifecycle for dynamic ids - #26
Merged
Conversation
Dynamic route ids need a deletion path. Factory lookup stays id-based, so the registry now tracks committed consumers. Final eviction is deferred to avoid StrictMode replay and late commit races. Constraint: Factory hooks resolve stores by id without Provider or Scope ancestry Rejected: Evict stores from getStore; render allocation cannot prove commit Confidence: high Scope-risk: moderate Directive: Keep render allocation separate from committed-consumer eviction Tested: Registry and integration lifecycle suites before commit Not-tested: Memory profiling of never-committed abandoned renders
Roots and public hooks are the committed registry consumers. Retain the store in layout effects and release it during cleanup. Root attach errors release the held store before rethrowing. Constraint: Duplicate Root protection must remain unchanged for a factory id Rejected: Allow two Roots for one id; external hook ownership would be ambiguous Confidence: high Scope-risk: moderate Directive: Keep attach cleanup ordering before changing duplicate Root behavior Tested: Integration lifecycle suites before commit Not-tested: Native-device navigation memory profile
Dynamic route ids now follow committed consumer lifetime. Cover eviction, late retain, StrictMode, and duplicate Root cleanup. Document stable ids and the abandoned-render limitation in both languages. Constraint: Duplicate Roots for one factory id must remain rejected Rejected: Treat unique ids as item keys | deck ids identify factory-scoped stores Confidence: high Scope-risk: moderate Directive: Update lifecycle tests and docs together when changing registry ownership Tested: 15 Jest suites / 213 tests; lint; typecheck; format; builds; docs checks Not-tested: Memory profiling of fresh ids from renders that never commit
Stable route ids avoid concurrent Root collisions but need lifecycle cleanup. Explain the committed-consumer contract, duplicate-Root rule, and residual risk. Ship the behavior as a minor Changeset for downstream consumers. Constraint: Deck ids identify factory-scoped stores rather than card items Rejected: Hide the render-abort limitation | eager cleanup can race delayed commits Confidence: high Scope-risk: narrow Directive: Keep English, Korean, API, and multi-instance guidance aligned Tested: Docs typecheck/build and generated callout output Not-tested: Memory profiling of fresh ids from renders that never commit
🦋 Changeset detectedLatest commit: 47d4ac8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Deploying react-native-swipe-deck with
|
| Latest commit: |
b3ab7ef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://59ab23a5.react-native-swipe-deck.pages.dev |
| Branch Preview URL: | https://fix-swipe-deck-registry-life.react-native-swipe-deck.pages.dev |
saseungmin
marked this pull request as ready for review
August 11, 2026 11:49
saseungmin
changed the title
The change repairs internal registry retention without adding public API. Align the Changeset with this repository's patch convention for bug fixes. Constraint: Identity resets only after the final committed consumer releases the id Rejected: Minor bump | no new public API or additive user-facing surface Confidence: high Scope-risk: narrow Tested: Changeset status, Oxfmt check, and git diff check
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3ab7efe07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Observe canonical store identity before committed hooks and Roots retain it. This rebases interrupted renders when another store has claimed the same id. The mismatch guard remains an internal invariant. Constraint: Providerless factory hooks resolve decks globally by id Rejected: Ignore stale retain conflicts | consumers would read an orphaned store Rejected: Per-id numeric generations | adds tombstones without better identity checks Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep identity snapshots pure and notify subscribers outside render Tested: Jest 16 suites / 216 tests; typecheck; lint; format; library and docs builds Not-tested: React 18 runtime matrix Not-tested: Fresh-id aborted renders retain the documented cleanup limitation
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SwipeDeck.Rootor public hook consumers use an idWhy
Apps can mount multiple screens from one
createSwipeDeck()factory by assigning each screen a stable route-scoped id. The registry previously had no deletion path, so every dynamic id kept its store, actions, events, and interaction values for the factory lifetime after the screen unmounted.Root cause
getStore(id)inserted stores into a factory-owned Map, but Roots and public hooks did not report committed ownership or cleanup. The registry therefore could not distinguish an active id from one whose final consumer had unmounted.Behavior
The duplicate-Root invariant is unchanged: two simultaneous Roots from the same factory still require distinct ids. Store identity remains stable while at least one committed Root or hook retains the id. After the final release and guarded deferred eviction, a later consumer receives a fresh store.
A fresh id read by a render that never commits remains a documented best-effort cleanup limitation. Eagerly evicting that entry could race a delayed concurrent commit.
Validation
pnpm test --runInBand --watchman=false— 15 suites, 213 tests passedpnpm lintpnpm typecheckpnpm format:checkpnpm clean && pnpm buildpnpm docs:typecheckpnpm docs:build