-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
chore: update svelte examples and tests from runes PR #9692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughDocumentation updates change initialIsOpen type annotations from Boolean to boolean across frameworks. Svelte docs/examples switch API endpoints to relative paths. Several Svelte example configs have comment/whitespace cleanup. One SSR load function becomes synchronous. Tests adjust output and expectations for mutation failure reason and mutation state rendering. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant SvelteSSR as +layout.ts (SSR)
participant QueryClient as QueryClient
User->>SvelteSSR: Request page
Note over SvelteSSR: Synchronous load() returns data
SvelteSSR->>QueryClient: new QueryClient()
SvelteSSR-->>User: { queryClient }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
View your CI Pipeline Execution ↗ for commit 8f45f04
☁️ Nx Cloud last updated this comment at |
Sizes for commit 8f45f04:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9692 +/- ##
===========================================
+ Coverage 46.38% 89.20% +42.82%
===========================================
Files 214 18 -196
Lines 8488 176 -8312
Branches 1927 31 -1896
===========================================
- Hits 3937 157 -3780
+ Misses 4108 18 -4090
+ Partials 443 1 -442
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/svelte-query/tests/useMutationState/BaseExample.svelte (2)
26-27
: Confirm calling mutate on the store value ($store) vs the store objectTypical Svelte Query usage is
successMutation.mutate()
(no$
), while$successMutation
is usually the state value. If methods are exposed on the store object (not the value), the current calls might fail. If intentional in this branch, ignore; otherwise, prefer the non-$
form to avoid unnecessary subscriptions in handlers.Apply if appropriate:
-<button on:click={() => $successMutation.mutate()}>Success</button> -<button on:click={() => $errorMutation.mutate()}>Error</button> +<button on:click={() => successMutation.mutate()}>Success</button> +<button on:click={() => errorMutation.mutate()}>Error</button>
29-31
: Stabilize selector surface for tests (optional)Relying on the literal "Data: ..." string couples tests to UI copy. Consider a test id to decouple formatting.
-<div> - Data: {JSON.stringify($mutationState.map((state) => state.status))} -</div> +<div data-testid="mutation-state"> + {JSON.stringify($mutationState.map((state) => state.status))} +</div>If adopted, update tests to assert via
getByTestId('mutation-state')
.packages/svelte-query/tests/useMutationState/useMutationState.test.ts (3)
36-45
: Good a11y query upgrade; consider async assertions to reduce flakinessSwitch to
getByRole
is solid. For DOM updates driven by timers and promises,findByText
is more robust thangetByText
.-expect(rendered.getByText('Data: ["success"]')).toBeInTheDocument() +await rendered.findByText('Data: ["success"]') ... -expect(rendered.getByText('Data: ["success","error"]')).toBeInTheDocument() +await rendered.findByText('Data: ["success","error"]')Also, verify that the ordering from
useMutationState
is deterministic so the array order assertion won’t flake.
71-80
: Filter test reads well; prefer async query for stabilityUse
findByText
to await DOM updates post timers.-expect(rendered.getByText('Data: []')).toBeInTheDocument() +await rendered.findByText('Data: []') ... -expect(rendered.getByText('Data: ["error"]')).toBeInTheDocument() +await rendered.findByText('Data: ["error"]')
106-115
: Key-filter test is clear; use async assertionsSame async assertion tweak to avoid timing flakes.
-expect(rendered.getByText('Data: ["success"]')).toBeInTheDocument() +await rendered.findByText('Data: ["success"]') ... -expect(rendered.getByText('Data: ["success"]')).toBeInTheDocument() +await rendered.findByText('Data: ["success"]')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
docs/framework/react/devtools.md
(1 hunks)docs/framework/solid/devtools.md
(1 hunks)docs/framework/svelte/devtools.md
(1 hunks)docs/framework/svelte/reactivity.md
(2 hunks)docs/framework/vue/devtools.md
(1 hunks)examples/svelte/auto-refetching/src/routes/+page.svelte
(3 hunks)examples/svelte/auto-refetching/svelte.config.js
(0 hunks)examples/svelte/basic/svelte.config.js
(0 hunks)examples/svelte/optimistic-updates/src/routes/+page.svelte
(1 hunks)examples/svelte/optimistic-updates/src/routes/api/data/+server.ts
(1 hunks)examples/svelte/optimistic-updates/svelte.config.js
(0 hunks)examples/svelte/playground/svelte.config.js
(0 hunks)examples/svelte/simple/svelte.config.js
(0 hunks)examples/svelte/ssr/src/routes/+layout.ts
(1 hunks)examples/svelte/ssr/svelte.config.js
(0 hunks)examples/svelte/star-wars/src/routes/+page.svelte
(1 hunks)examples/svelte/star-wars/svelte.config.js
(0 hunks)packages/svelte-query/tests/createMutation/FailureExample.svelte
(1 hunks)packages/svelte-query/tests/createMutation/createMutation.test.ts
(1 hunks)packages/svelte-query/tests/useMutationState/BaseExample.svelte
(1 hunks)packages/svelte-query/tests/useMutationState/useMutationState.test.ts
(3 hunks)
💤 Files with no reviewable changes (7)
- examples/svelte/star-wars/svelte.config.js
- examples/svelte/optimistic-updates/svelte.config.js
- examples/svelte/playground/svelte.config.js
- examples/svelte/simple/svelte.config.js
- examples/svelte/ssr/svelte.config.js
- examples/svelte/basic/svelte.config.js
- examples/svelte/auto-refetching/svelte.config.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (12)
examples/svelte/star-wars/src/routes/+page.svelte (1)
5-5
: Formatting tweak looks goodInline closing parenthesis improves readability without altering behavior.
examples/svelte/ssr/src/routes/+layout.ts (1)
5-16
: Synchronous load is appropriate here.Dropping
async
keeps the signature valid and avoids needless promise wrapping since the body does not await anything.docs/framework/solid/devtools.md (1)
71-71
: Primitiveboolean
matches TS conventionsGood call switching the docs to the lowercase primitive type; it aligns with TypeScript usage and the other framework docs.
docs/framework/react/devtools.md (1)
77-77
: Consistent primitive type annotationThe lowercase
boolean
is the correct TypeScript primitive and keeps this options list consistent across the docs.docs/framework/vue/devtools.md (1)
64-64
: Vue docs now mirror the TS primitiveThanks for updating this to
boolean
; it matches the actual prop typing and the rest of the documentation.docs/framework/svelte/devtools.md (1)
64-64
: Svelte options reflect the right boolean typeLowercase
boolean
is the right primitive and keeps parity with the other framework docs.examples/svelte/optimistic-updates/src/routes/api/data/+server.ts (1)
9-9
: Type alias tweak looks good
Array<Todo>
maintains identical semantics to the previousTodo[]
, so no runtime impact. 👍examples/svelte/optimistic-updates/src/routes/+page.svelte (1)
23-37
: Relative endpoint improves portabilitySwitching to the relative
/api/data
keeps local/dev parity while avoiding hard-coded hostnames. All fetching logic still lines up with the server route.packages/svelte-query/tests/createMutation/createMutation.test.ts (1)
89-97
: Expectation aligns with updated fallback rendering
FailureExample.svelte
now renders the nullish failure reason as the literal stringundefined
, so this revised assertion keeps the test in sync with the component output.packages/svelte-query/tests/createMutation/FailureExample.svelte (1)
22-23
: Rendering tweak matches mutation state semanticsDisplaying the raw
failureCount
and falling back to the string'undefined'
for a nullishfailureReason
mirrors TanStack Query’s mutation state, so the UI now reflects the state machine more precisely and stays consistent with the updated test.examples/svelte/auto-refetching/src/routes/+page.svelte (1)
23-26
: Great call hardening the mutation URL.Using
encodeURIComponent
before interpolating user input into the fetch query string prevents accidental breakage (and avoids surprise characters in the request). Nicely done keeping the rest of the chain untouched.docs/framework/svelte/reactivity.md (1)
18-39
: Docs update looks solid.Switching the snippets to the shared
/api/data
endpoint keeps the walkthrough aligned with the updated example and avoids localhost-only assumptions.
🎯 Changes
Takes some of the changes from the svelte runes adapter branch and applies them here
✅ Checklist
pnpm test:pr
.🚀 Release Impact
Summary by CodeRabbit
Documentation
Chores
Style
Tests