feat(nuxt): add useRouteQuery composable
#33496
Conversation
|
|
WalkthroughAdds a new composable useRouteQuery implemented in packages/nuxt/src/app/composables/route-query.ts and re-exports it from packages/nuxt/src/app/composables/index.ts. The composable exposes types and options for reactive route query state with optional transform functions, supports client-side batched updates and server-side navigation, and cleans up on scope disposal. The import presets are updated to include the new composable, and tests are updated to include useRouteQuery in the tested composables list. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/nuxt/src/app/composables/route-query.ts (3)
118-124: Avoidqueryshadowing for clarityLocal
query(current value) is shadowed by destructuredqueryfromroute, which is confusing. Rename the destructured one.Apply this diff:
- const { params, query, hash } = route + const { params, query: routeQuery, hash } = route router[toValue(mode)]({ params, - query: { ...query, ...newQueries }, + query: { ...routeQuery, ...newQueries }, hash, })
4-6: Use Vue Router’s query types (LocationQueryValueRaw)Route query types differ from route params. Using
LocationQueryValueRaw(and its array) is more accurate thanRouteParamValueRaw | string[].Apply this diff:
-import type { RouteParamValueRaw, Router } from 'vue-router' +import type { LocationQueryValueRaw, Router } from 'vue-router' -export type RouteQueryValueRaw = RouteParamValueRaw | string[] +export type RouteQueryValueRaw = LocationQueryValueRaw | LocationQueryValueRaw[]
14-15: Minor type ergonomics
- Accept
modeas a getter too for parity withdefaultValue.- Keep the ref typed by using
customRef<K>.Apply this diff:
- mode?: MaybeRef<'replace' | 'push'> + mode?: MaybeRefOrGetter<'replace' | 'push'>- const proxy = customRef<any>((track, trigger) => { + const proxy = customRef<K>((track, trigger) => {Also applies to: 78-78
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/nuxt/src/app/composables/index.ts(1 hunks)packages/nuxt/src/app/composables/route-query.ts(1 hunks)packages/nuxt/src/imports/presets.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/app/composables/index.tspackages/nuxt/src/imports/presets.tspackages/nuxt/src/app/composables/route-query.ts
🧬 Code graph analysis (1)
packages/nuxt/src/app/composables/route-query.ts (1)
packages/nuxt/src/app/composables/index.ts (4)
useRouteQuery(34-34)useRoute(16-16)useRouter(16-16)navigateTo(16-16)
⏰ 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). (19)
- GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: release-pkg-pr-new
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: test-benchmark
- GitHub Check: test-size
- GitHub Check: typecheck (windows-latest, bundler)
🔇 Additional comments (2)
packages/nuxt/src/app/composables/index.ts (1)
34-34: Public export LGTMExport path and naming are consistent with the new composable.
packages/nuxt/src/imports/presets.ts (1)
104-107: Preset addition looks correct; check for auto‑import collisionsPath and inclusion are consistent. If users also enable @vueuse/nuxt (which auto-imports @vueuse/router/useRouteQuery), there could be an import name collision/precedence issue. Please confirm desired precedence and add a note to docs if we intend to shadow VueUse.
| redirectCode: mode === 'replace' ? 301 : 302, | ||
| }, |
There was a problem hiding this comment.
Avoid permanent redirects for SSR query updates
Using 301 on replace can be cached by browsers/CDNs, causing sticky redirects. Prefer non‑permanent (302/307) by default.
Apply this diff:
- redirectCode: mode === 'replace' ? 301 : 302,
+ redirectCode: 302,Optionally accept a user‑configurable redirect code in options if needed.
📝 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.
| redirectCode: mode === 'replace' ? 301 : 302, | |
| }, | |
| redirectCode: 302, | |
| }, |
🤖 Prompt for AI Agents
In packages/nuxt/src/app/composables/route-query.ts around lines 103–104, the
code currently uses a 301 redirect for mode === 'replace', which can be cached
by browsers/CDNs and cause sticky permanent redirects; change the default to a
non‑permanent code (e.g., 302 or 307) when mode === 'replace' to avoid caching
issues, and update the logic to allow an optional user‑provided redirectCode in
the function/options so callers can override the default when needed.
CodSpeed Performance ReportMerging #33496 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/nuxt/composables.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
test/nuxt/composables.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
test/nuxt/composables.test.ts
⏰ 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). (18)
- GitHub Check: test-fixtures (windows-latest, dev, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, webpack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-benchmark
- GitHub Check: release-pkg-pr-new
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: test-size
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: code
| 'useServerHeadSafe', | ||
| 'useServerSeoMeta', | ||
| 'usePreviewMode', | ||
| 'useRouteQuery', |
There was a problem hiding this comment.
🧩 Analysis chain
Add test coverage for useRouteQuery before merging.
The new composable is currently skipped from testing. Whilst this is acceptable for a POC, test coverage should be added before merging to the main branch, especially since the composable handles reactive route query state and SSR navigation.
Run the following script to check if tests exist elsewhere for useRouteQuery:
Would you like me to help generate test cases for this composable based on the implementation?
🏁 Script executed:
#!/bin/bash
# Description: Search for existing tests for useRouteQuery
# Search for test files mentioning useRouteQuery
rg -n --type=ts --type=js 'useRouteQuery' -g '*{test,spec}*'Length of output: 56
Add unit tests for useRouteQuery
The new composable is currently skipped in the test suite; add vitest coverage before merging.
🤖 Prompt for AI Agents
In test/nuxt/composables.test.ts around line 130, the new composable
useRouteQuery is currently skipped from tests; add Vitest unit tests that import
useRouteQuery and exercise its behavior: mock the router/route query (using
vi.mock for vue-router or Nuxt route helpers) to provide initial query values,
mount a small test component or call the composable inside a setup() test
harness to read the returned reactive ref, assert it reflects the expected
value, then mutate the mocked route query and assert the ref updates
accordingly; remove the skip and ensure tests cover edge cases (missing key,
default value, and updates) so the composable is actually covered by the test
suite.
|
if you use vueuse's version, does Nuxt redirect anyway? (I think it might). |
|
Using VueUse's version nothing happened during SSR when I needed to assign a different value to a query param, but if you think I have missed anything, feel free to check it yourself! I am currently doing it like so in my projects: Taking care of possible inifinite redirects is up to the user. |
🔗 Linked issue
related #24788
📚 Description
POC of Nuxt-native
useRouteQuerycomposable - thoughts welcome!During SSR, when a value is assigned to a ref it returns, it will perform a network redirect using Nuxt's
navigateTo. The remaining part is taken from VueUse.