Add Lit Store Adapter#320
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR introduces ChangesLit Store Package Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Sequence Diagram(s)sequenceDiagram
participant Client as Lit Host
participant Controller as TanStackStoreSelector
participant Store as TanStack Store
Client->>Controller: hostUpdate()
Controller->>Store: get() -> snapshot
Controller->>Controller: selector(snapshot) -> selected
Controller->>Controller: compare(lastSelected, selected)
alt changed
Controller->>Client: requestUpdate()
end
Client->>Controller: hostDisconnected()
Controller->>Store: unsubscribe()
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
View your CI Pipeline Execution ↗ for commit af10169
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview1 package(s) bumped directly, 0 bumped as dependents. 🟩 Patch bumps
|
@tanstack/angular-store
@tanstack/lit-store
@tanstack/preact-store
@tanstack/react-store
@tanstack/solid-store
@tanstack/store
@tanstack/svelte-store
@tanstack/vue-store
commit: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/lit-store/tests/selector.test.ts (3)
10-10: ⚡ Quick winAvoid sharing a single
userEventinstance across tests.Creating
userat module scope can introduce cross-test coupling; instantiate per test (or inbeforeEach) for isolation.Proposed change
-const user = userEvent.setup() - describe('Lit Store Tests', () => { it('should update when a store is selected with no selector', async () => { + const user = userEvent.setup() const counter = createStore(0) @@ it('should ignore updates when a store is selected with a selector', async () => { + const user = userEvent.setup() const counter = createStore({ count: 0, ignore: 1 })🤖 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-store/tests/selector.test.ts` at line 10, The test file creates a module-scoped user via userEvent.setup() assigned to the variable user which can cause cross-test coupling; move the userEvent.setup() call into each test or a beforeEach so each test gets its own user instance (replace the module-level const user with a per-test instantiation of userEvent.setup() or create it in a beforeEach and assign to the same variable name).
28-28: ⚡ Quick winUse unique custom-element tags per test case.
Reusing
'test-form'in both tests can make behavior order-dependent in the custom element registry; unique tags keep tests deterministic.Proposed change
-defineOnce('test-form', TestForm) -const element = await mount<TestForm>('test-form') +defineOnce('test-form-basic', TestForm) +const element = await mount<TestForm>('test-form-basic') @@ -defineOnce('test-form', TestForm) -const element = await mount<TestForm>('test-form') +defineOnce('test-form-selector', TestForm) +const element = await mount<TestForm>('test-form-selector')Also applies to: 72-72
🤖 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-store/tests/selector.test.ts` at line 28, The test reuses the same custom element tag ('test-form') causing registry/order-dependent failures; update each test to register a uniquely named tag when calling defineOnce (e.g., 'test-form-1', 'test-form-2' or otherwise unique per test) while still passing the same TestForm class to defineOnce so the element registration is deterministic; locate the defineOnce('test-form', TestForm) calls in the tests and change the tag strings to unique identifiers for each separate test case.
12-12: ⚡ Quick winMake
describecallback synchronous.Line 12 uses
asyncondescribe, which is non-idiomatic in Vitest and can make suite registration behavior harder to reason about.Proposed change
-describe('Lit Store Tests', async () => { +describe('Lit Store Tests', () => {🤖 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-store/tests/selector.test.ts` at line 12, The test suite callback passed to describe in selector.test.ts is marked async (describe('Lit Store Tests', async () => ...)), which is non-idiomatic; remove the async modifier from the describe callback and move any asynchronous setup/awaits into proper hooks (e.g., beforeAll, beforeEach) or into individual it/test callbacks so suite registration stays synchronous and tests still perform needed async work.packages/lit-store/package.json (1)
40-48: ⚡ Quick winAlign exports with declared dual-format build output.
The package builds CJS + ESM, but
exportsexposes only an import/default path. If dual-mode support is intended, add arequirecondition; otherwise drop CJS build to avoid dead artifacts.🤖 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-store/package.json` around lines 40 - 48, The package.json "exports" field currently provides only an "import" and "default" path (symbols: "exports", "import", "default") while the build produces both ESM and CJS; update the "exports" to add a "require" condition pointing to the CJS artifact (e.g., add a "require": "./dist/index.cjs" alongside the existing keys) so consumers using require() resolve correctly, or if you intend to ship only ESM, remove the CJS build output and any CJS references from the build pipeline to avoid dead artifacts; ensure the added "require" entry matches the actual CJS filename emitted by the build.
🤖 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-store/tests/selector.test.ts`:
- Around line 79-80: The queried element in the getIgnore helper is incorrectly
typed as HTMLParagraphElement; update the query in selector.test.ts (the
getIgnore function that calls
element.shadowRoot!.querySelector<HTMLParagraphElement>('#ignore')) to use
HTMLButtonElement instead so TypeScript reflects that the DOM node with id
"ignore" is a <button> and provides correct DOM API typings.
In `@packages/lit-store/tsconfig.docs.json`:
- Around line 4-6: The tsconfig.docs.json contains an incorrect "paths" entry
mapping "@tanstack/form-core" (copy-paste error); fix it by either removing the
"paths" mapping entirely if not needed, or replace the incorrect key with the
correct package name "@tanstack/lit-store" and point it to the local source
directory used for lit-store (update the value accordingly) so the "paths" block
reflects the actual package being built.
---
Nitpick comments:
In `@packages/lit-store/package.json`:
- Around line 40-48: The package.json "exports" field currently provides only an
"import" and "default" path (symbols: "exports", "import", "default") while the
build produces both ESM and CJS; update the "exports" to add a "require"
condition pointing to the CJS artifact (e.g., add a "require":
"./dist/index.cjs" alongside the existing keys) so consumers using require()
resolve correctly, or if you intend to ship only ESM, remove the CJS build
output and any CJS references from the build pipeline to avoid dead artifacts;
ensure the added "require" entry matches the actual CJS filename emitted by the
build.
In `@packages/lit-store/tests/selector.test.ts`:
- Line 10: The test file creates a module-scoped user via userEvent.setup()
assigned to the variable user which can cause cross-test coupling; move the
userEvent.setup() call into each test or a beforeEach so each test gets its own
user instance (replace the module-level const user with a per-test instantiation
of userEvent.setup() or create it in a beforeEach and assign to the same
variable name).
- Line 28: The test reuses the same custom element tag ('test-form') causing
registry/order-dependent failures; update each test to register a uniquely named
tag when calling defineOnce (e.g., 'test-form-1', 'test-form-2' or otherwise
unique per test) while still passing the same TestForm class to defineOnce so
the element registration is deterministic; locate the defineOnce('test-form',
TestForm) calls in the tests and change the tag strings to unique identifiers
for each separate test case.
- Line 12: The test suite callback passed to describe in selector.test.ts is
marked async (describe('Lit Store Tests', async () => ...)), which is
non-idiomatic; remove the async modifier from the describe callback and move any
asynchronous setup/awaits into proper hooks (e.g., beforeAll, beforeEach) or
into individual it/test callbacks so suite registration stays synchronous and
tests still perform needed async work.
🪄 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: 76c6ba1f-ed31-4927-a018-afb5f451c1ce
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
packages/lit-store/eslint.config.jspackages/lit-store/package.jsonpackages/lit-store/src/index.tspackages/lit-store/src/tan-stack-store-selector.tspackages/lit-store/tests/selector.test.tspackages/lit-store/tests/test-setup.tspackages/lit-store/tests/utils.tspackages/lit-store/tsconfig.build.jsonpackages/lit-store/tsconfig.docs.jsonpackages/lit-store/tsconfig.jsonpackages/lit-store/tsdown.config.tspackages/lit-store/vitest.config.ts
| "paths": { | ||
| "@tanstack/form-core": ["../form-core/src"] | ||
| } |
There was a problem hiding this comment.
Fix incorrect package path mapping.
This configuration maps @tanstack/form-core, but this is the @tanstack/lit-store package, not a form package. This appears to be a copy-paste error.
🔧 Proposed fix
Either remove the paths mapping entirely if not needed, or map the correct package:
"compilerOptions": {
"paths": {
- "@tanstack/form-core": ["../form-core/src"]
+ "@tanstack/store-core": ["../store-core/src"]
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "paths": { | |
| "@tanstack/form-core": ["../form-core/src"] | |
| } | |
| "paths": { | |
| "@tanstack/store-core": ["../store-core/src"] | |
| } |
🤖 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-store/tsconfig.docs.json` around lines 4 - 6, The
tsconfig.docs.json contains an incorrect "paths" entry mapping
"@tanstack/form-core" (copy-paste error); fix it by either removing the "paths"
mapping entirely if not needed, or replace the incorrect key with the correct
package name "@tanstack/lit-store" and point it to the local source directory
used for lit-store (update the value accordingly) so the "paths" block reflects
the actual package being built.
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-store/src/tan-stack-store-atom.ts`:
- Line 17: The JSDoc example uses the wrong class name `TanStackAtom`; update
the example to use the exported class `TanStackStoreAtom` so the copy/paste
snippet is correct (e.g., replace `TanStackAtom` with `TanStackStoreAtom` in the
JSDoc example near the `#count = new TanStackAtom(this, () => countAtom)` line).
🪄 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: 9c1fc1d8-8f3e-4749-a671-d48d9825a41b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
.changeset/polite-hats-beg.mddocs/config.jsondocs/framework/lit/quick-start.mdexamples/lit/simple/.eslintrc.cjsexamples/lit/simple/.gitignoreexamples/lit/simple/README.mdexamples/lit/simple/index.htmlexamples/lit/simple/package.jsonexamples/lit/simple/src/index.tsexamples/lit/simple/tsconfig.jsonpackages/lit-store/package.jsonpackages/lit-store/src/index.tspackages/lit-store/src/tan-stack-store-atom.tspackages/lit-store/tests/atom.test.tspackages/lit-store/tests/selector.test.tspackages/lit-store/tests/utils.tspnpm-workspace.yamlscripts/generate-docs.ts
✅ Files skipped from review due to trivial changes (9)
- .changeset/polite-hats-beg.md
- examples/lit/simple/.gitignore
- pnpm-workspace.yaml
- examples/lit/simple/README.md
- examples/lit/simple/package.json
- examples/lit/simple/index.html
- examples/lit/simple/.eslintrc.cjs
- docs/framework/lit/quick-start.md
- packages/lit-store/tests/selector.test.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This PR adds Lit to the supported frameworks
Summary by CodeRabbit
New Features
Documentation
Examples
Tests
Chores