Skip to content

refactor(KeepAlive): scope wrapChildren props, drop as any in getExisting#10

Merged
chiefcll merged 2 commits into
mainfrom
refactor/keepalive-cleanup
May 14, 2026
Merged

refactor(KeepAlive): scope wrapChildren props, drop as any in getExisting#10
chiefcll merged 2 commits into
mainfrom
refactor/keepalive-cleanup

Conversation

@chiefcll
Copy link
Copy Markdown
Contributor

Summary

Two small cleanups to src/primitives/KeepAlive.tsx. No behavior change.

1. Stop leaking KeepAlive-level config onto the Lightning view

wrapChildren did:

<view
  {...props}
  preserve
  onRemove={onRemove}
  onRender={onRender}
  forwardFocus={0}
  transition={transition}
/>

props is s.ParentProps<KeepAliveProps>, so the spread also pushed id, shouldDispose, and the original unchained onRemove / onRender / transition onto the view before the targeted overrides re-set them. Harmless for id, but shouldDispose is purely KeepAlive-level — it has no business landing on a Lightning element. Replace the spread with explicit props the view needs and insert props.children directly.

2. Reflect that KeepAliveElement is populated lazily

getExisting constructs a partial entry holding only id + the isAlive signal, and only populates owner / children / dispose once the route mounts or preload runs. Today the type marks those three fields as required, and getExisting resorts to as any. Mark them optional on KeepAliveElement (matching reality), drop the cast, and tighten the existing casts at call-sites to as unknown as ElementNode | undefined so optional chaining can work honestly.

Test plan

  • tsc --noEmit clean (pre-existing errors elsewhere in the repo are unaffected).
  • eslint src/primitives/KeepAlive.tsx clean.
  • Manual: KeepAlive + KeepAliveRoute mount/unmount/back-nav cycles unchanged.

🤖 Generated with Claude Code

chiefcll and others added 2 commits May 13, 2026 20:46
…isting

Two small cleanups to KeepAlive.tsx:

1. Stop leaking KeepAlive-level config onto the Lightning view
   `wrapChildren` spread the full props bag onto the inner `<view>` and
   then overrode the handful it actually wanted (`onRemove`, `onRender`,
   `transition`, `preserve`, `forwardFocus`). That meant the view was
   also receiving `id`, `shouldDispose`, and the *original* (unchained)
   `onRemove` / `onRender` / `transition` before they were re-set —
   harmless for `id`, but `shouldDispose` is not a view-level concern
   and noisily lands on the Lightning element. Switch to passing only
   the props the view needs, and insert `props.children` explicitly.

2. Reflect that KeepAliveElement is built up lazily
   `getExisting` constructs a partial entry holding only `id` + the
   `isAlive` signal until the route mounts; it then `as any`-casts to
   satisfy a type that declared `owner`, `children`, and `dispose` as
   required. Mark those three fields optional on `KeepAliveElement`
   (matching reality), drop the `as any`, and tighten the casts at
   call-sites: `as unknown as ElementNode | undefined` so optional
   chaining works without lying about the value's presence.

No behavior change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@chiefcll chiefcll merged commit 8c2b482 into main May 14, 2026
1 check 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.

1 participant