feat: use lit-store internally instead of reyling on vanilla store dependency#6267
feat: use lit-store internally instead of reyling on vanilla store dependency#6267fredericbahr wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe PR migrates lit-table from ChangesStore package migration and selector refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/lit-table/src/TableController.ts`:
- Around line 217-218: Cache and reuse selector instances per source/selector
pair to avoid creating a new TanStackStoreSelector on every Subscribe call: add
a private two-level cache field (e.g., _selectorCache: WeakMap<any, Map<any,
TanStackStoreSelector<unknown>>> ) on TableController, update
_getOrCreateSelector(source, selector) to look up and return an existing
instance from _selectorCache[source].get(selector) or create, store, and return
a new one; ensure keys use the raw selector object/function (handle undefined
selectors consistently). Also update hostDisconnected (or appropriate teardown)
to remove the source entry from _selectorCache so entries can be
garbage-collected when a source is gone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a4fff87a-9368-42bf-8177-7772f2a4b78d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/lit-table/package.jsonpackages/lit-table/src/TableController.tspackages/lit-table/src/reactivity.ts
| const storeSelector: TanStackStoreSelector<unknown> = | ||
| this._getOrCreateSelector(source, props.selector) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate TanStackStoreSelector implementation =="
selector_files="$(fd -i 'TanStackStoreSelector.*\.ts$' || true)"
if [ -z "${selector_files}" ]; then
echo "No TanStackStoreSelector source file found in-repo (may be external package)."
else
while IFS= read -r f; do
echo "---- $f ----"
rg -n -C3 'class\s+TanStackStoreSelector|addController\(|subscribe\(|unsubscribe\(' "$f"
done <<< "${selector_files}"
fi
echo
echo "== Inspect creation sites in TableController =="
rg -n -C3 '_getOrCreateSelector|new TanStackStoreSelector|storeSelector\.value' packages/lit-table/src/TableController.tsRepository: TanStack/table
Length of output: 1193
🏁 Script executed:
cat -n packages/lit-table/src/TableController.ts | head -n 320 | tail -n 100Repository: TanStack/table
Length of output: 3667
🏁 Script executed:
rg -n 'import.*TanStackStoreSelector|from.*TanStackStoreSelector' packages/lit-table/src/TableController.tsRepository: TanStack/table
Length of output: 120
Cache selector instances; new instance created on every Subscribe call.
Line 279 creates a fresh TanStackStoreSelector on each Subscribe invocation. The TODO comment on line 269 acknowledges this concern. In render-heavy usage, this accumulates controller and subscription instances without cleanup, since hostDisconnected only cleans up the main store subscriptions, not these selector instances.
Implement a two-level cache using WeakMap<source, Map<selector, instance>> to reuse instances per (source, selector) pair. The WeakMap ensures the cache is garbage-collected when sources are no longer referenced.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lit-table/src/TableController.ts` around lines 217 - 218, Cache and
reuse selector instances per source/selector pair to avoid creating a new
TanStackStoreSelector on every Subscribe call: add a private two-level cache
field (e.g., _selectorCache: WeakMap<any, Map<any,
TanStackStoreSelector<unknown>>> ) on TableController, update
_getOrCreateSelector(source, selector) to look up and return an existing
instance from _selectorCache[source].get(selector) or create, store, and return
a new one; ensure keys use the raw selector object/function (handle undefined
selectors consistently). Also update hostDisconnected (or appropriate teardown)
to remove the source entry from _selectorCache so entries can be
garbage-collected when a source is gone.
Summary by CodeRabbit
@tanstack/lit-storedependency to version 0.13.2.