test(svelte-query/queryOptions): add runtime test for identity function#10178
test(svelte-query/queryOptions): add runtime test for identity function#10178sukvvon wants to merge 7 commits intoTanStack:mainfrom
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Vitest unit test that asserts Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit fc06f53
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/svelte-query/tests/queryOptions/queryOptions.svelte.test.ts (1)
1-5: The.svelte.test.tsextension doesn't match the test's contents.This test is a pure unit test with no Svelte-specific code (no
flushSync,$state, component mounting, etc.). While the entire svelte-query test suite consistently uses.svelte.test.tsfor uniformity, this file could be renamed to.test.tsfor accuracy, since other files in the package that actually use Svelte runtime features share the same extension. Consider renaming to.test.tsunless there's a specific reason to maintain the package-wide.svelte.test.tsconvention.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/svelte-query/tests/queryOptions/queryOptions.svelte.test.ts` around lines 1 - 5, The test file uses only plain unit tests for the exported function queryOptions and therefore should be renamed from queryOptions.svelte.test.ts to queryOptions.test.ts to reflect that it contains no Svelte-specific code; locate the test that imports queryOptions (symbol: queryOptions) and rename the file extension, and if any project scripts or test-indexing rely on the .svelte.test.ts convention update them accordingly so the test runner still discovers the renamed file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/svelte-query/tests/queryOptions/queryOptions.svelte.test.ts`:
- Line 11: The test currently asserts structural equality but should assert
reference identity; update the assertion in the test for queryOptions to use
toBe instead of toStrictEqual so it verifies that queryOptions(object) returns
the exact same object reference (i.e.,
expect(queryOptions(object)).toBe(object)); locate the assertion referencing
queryOptions in the queryOptions.svelte.test.ts and replace the matcher
accordingly.
---
Nitpick comments:
In `@packages/svelte-query/tests/queryOptions/queryOptions.svelte.test.ts`:
- Around line 1-5: The test file uses only plain unit tests for the exported
function queryOptions and therefore should be renamed from
queryOptions.svelte.test.ts to queryOptions.test.ts to reflect that it contains
no Svelte-specific code; locate the test that imports queryOptions (symbol:
queryOptions) and rename the file extension, and if any project scripts or
test-indexing rely on the .svelte.test.ts convention update them accordingly so
the test runner still discovers the renamed file.
packages/svelte-query/tests/queryOptions/queryOptions.svelte.test.ts
Outdated
Show resolved
Hide resolved
packages/svelte-query/tests/queryOptions/queryOptions.svelte.test.ts
Outdated
Show resolved
Hide resolved
…otation due to overload mismatch
🎯 Changes
Add a runtime test for
queryOptionsidentity function in@tanstack/svelte-query, matching the existing test in@tanstack/react-query.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit