Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new Svelte client package: package manifest, build/test configs, Svelte/Vite tooling, a TypeScript wrapper around InstantDB core with reactive hooks (queries, auth, rooms/presence/typing), unit tests, and publish mapping. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SvelteComponent as Svelte Component
participant InstantSvelteDB as InstantSvelteDatabase
participant Reactor as Core Reactor
participant CoreDB as InstantDB Core
User->>SvelteComponent: mount
SvelteComponent->>InstantSvelteDB: init() / construct
InstantSvelteDB->>CoreDB: core_init / create core instance
InstantSvelteDB-->>SvelteComponent: db instance
rect rgba(100, 200, 255, 0.5)
Note over SvelteComponent,Reactor: useQuery flow
SvelteComponent->>InstantSvelteDB: useQuery(query)
InstantSvelteDB->>SvelteComponent: { loading: true }
InstantSvelteDB->>Reactor: subscribeQuery(query, callback)
Reactor-->>InstantSvelteDB: onData(result)
InstantSvelteDB-->>SvelteComponent: update data/loading
end
rect rgba(200, 255, 100, 0.5)
Note over SvelteComponent,Reactor: presence/typing flow
SvelteComponent->>InstantSvelteDB: db.room(type,id).usePresence()
InstantSvelteDB->>Reactor: room.subscribePresence(callback)
SvelteComponent->>InstantSvelteDB: publishPresence(data)
InstantSvelteDB->>Reactor: reactor.publishPresence(topic, data)
Reactor-->>InstantSvelteDB: broadcast presence to peers
InstantSvelteDB-->>SvelteComponent: presence updates
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Without this entry the svelte package version doesn't get set before publish, causing CI to fail on first publish. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
View Vercel preview at instant-www-js-svelte-sdk-jsv.vercel.app. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
client/packages/svelte/src/tests/InstantSvelteDatabase.svelte.test.ts (1)
262-279: Use a validConnectionStatusvalue in the test.The test passes
'connected'to the status callback, but based on the core API, validConnectionStatusvalues are:'connecting','opened','authenticated','closed','errored'. Using'authenticated'would better document expected behavior.📝 Suggested fix
- statusCb!('connected'); + statusCb!('authenticated'); - expect(status.current).toBe('connected'); + expect(status.current).toBe('authenticated');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/svelte/src/tests/InstantSvelteDatabase.svelte.test.ts` around lines 262 - 279, The test uses an invalid connection string; update the value passed to the mocked subscription callback to a valid ConnectionStatus (e.g. replace the `'connected'` argument to statusCb in the "updates when connection status changes" test with `'authenticated'`) so the call from mockCore.subscribeConnectionStatus and the assertion against db.useConnectionStatus() use a proper ConnectionStatus value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/packages/svelte/src/lib/InstantSvelteDatabase.svelte.ts`:
- Around line 165-173: The branch that copies previous state only executes when
prev is truthy, leaving prior query data visible when switching to an uncached
query; update the logic around coerceQuery/coerceQuery(q) +
this.core._reactor.getPreviousResult(coerced) so that you always compute const
prevState = stateForResult(prev) (allowing prev to be undefined) and then assign
result.isLoading, result.data, result.pageInfo, and result.error from prevState
unconditionally (or explicitly set result = stateForResult(undefined) when prev
is falsy) so the UI resets immediately when switching to an uncached query.
In `@client/packages/svelte/src/lib/InstantSvelteRoom.svelte.ts`:
- Around line 185-193: The effect using $effect in InstantSvelteRoom.svelte.ts
currently calls room.core._reactor.publishPresence(room.type, room.id, data) but
does not return the publisher disposer and also uses deps.forEach((d) => (typeof
d === 'function' ? d() : d)) which returns values into forEach; change the deps
tracking to not return a value (e.g. use for (const d of deps) { if (typeof d
=== 'function') d(); } or call void on the result) and return the disposer from
room.core._reactor.publishPresence(...) so the effect cleans up the previous
publisher on deps/data changes and on unmount (mirror the behavior in
InstantReactRoom.ts).
---
Nitpick comments:
In `@client/packages/svelte/src/tests/InstantSvelteDatabase.svelte.test.ts`:
- Around line 262-279: The test uses an invalid connection string; update the
value passed to the mocked subscription callback to a valid ConnectionStatus
(e.g. replace the `'connected'` argument to statusCb in the "updates when
connection status changes" test with `'authenticated'`) so the call from
mockCore.subscribeConnectionStatus and the assertion against
db.useConnectionStatus() use a proper ConnectionStatus value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5c92b9e3-7da3-4049-85c2-9c92c9bd06fa
⛔ Files ignored due to path filters (1)
client/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
client/packages/svelte/.gitignoreclient/packages/svelte/package.jsonclient/packages/svelte/src/lib/InstantSvelteDatabase.svelte.tsclient/packages/svelte/src/lib/InstantSvelteRoom.svelte.tsclient/packages/svelte/src/lib/index.tsclient/packages/svelte/src/lib/version.tsclient/packages/svelte/src/tests/InstantSvelteDatabase.svelte.test.tsclient/packages/svelte/svelte.config.jsclient/packages/svelte/tsconfig.jsonclient/packages/svelte/vite.config.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| * const auth = db.useAuth(); | ||
| * // auth.isLoading, auth.user, auth.error | ||
| */ | ||
| useAuth = (): AuthState => { |
There was a problem hiding this comment.
same question on this and the stuff below re: the use primitive
|
All merged via #2367 |
Adds
@instantdb/sveltepackage with reactive hooks for Svelte 5, following the same composition-wrapper pattern as@instantdb/solidjsImplements
useQuery,useAuth,useUser,useConnectionStatus,useLocalId, and all room hooks (presence, topics, typing indicators) along with tests!Svelte 5 primer
Runes are Svelte 5's reactivity primitives (replacing Svelte 4's stores). The two we use:
$state(value)creates a reactive proxy. When you mutate properties on it (result.data = newData), anything reading those properties re-renders. This is why our subscription callbacks mutate individual properties rather than reassigning the whole object.$effect(() => { ... })is the equivalent of SolidJS'screateEffector React'suseEffect. It auto-tracks any reactive values read inside it and re-runs when they change. Returning a function from$effectregisters a cleanup (likeonCleanupin Solid)..svelte.tsfile extension is required for any file that uses runes. Regular.tsfiles will throwrune_outside_svelteat runtime. This is why the source files use.svelte.ts.Build uses
svelte-packageinstead oftshybecause.svelte.tsfiles need the Svelte compiler to process runes.tshy/tsccan't handle them.Key differences from SolidJS SDK
createSignal()returns[getter, setter]$state({})returns a mutable proxystate().data(call the getter)state.data(read the property)onCleanup(unsub)return unsubfrom$effectAccessor<string>(getter fn){ current: string }(wrapper object, since primitives can't be proxied)