feat(useQuery): add useQuery#55
Conversation
WalkthroughAdds a new useQuery composition utility with JSON-parsing helpers, unit tests, and Chinese documentation; exposes useQuery via the public exports barrel and inserts the new entry into the function index. No other modules or API signatures outside useQuery were changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as Component
participant UQ as useQuery()
participant OL as tryOnLoad
participant PP as processParams()
participant TP as tryParseJson()
U->>C: Open page
C->>UQ: call useQuery(key?, options?)
UQ->>OL: register onLoad handler
note right of OL #f9f9f9: On page load -> raw params
OL-->>UQ: rawParams
alt options.parseJson != false
UQ->>PP: processParams(rawParams)
loop each param
PP->>TP: tryParseJson(value)
TP-->>PP: parsed/original value
end
PP-->>UQ: processedParams
else options.parseJson == false
OL-->>UQ: use rawParams
end
UQ-->>C: returns { query: Ref, value: Computed }
C->>C: consume reactively
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/useQuery/index.md (4)
131-136: API return type mismatch with implementation.
useQuery()also returnsvalue(computed null when no key). Update the signature.Apply this diff:
function useQuery(): { - query: Ref<Record<string, any>>; + query: Ref<Record<string, any>>; + value: Ref<null>; };
176-178: Parameter type of tryParseJson is too narrow in docs.
Implementation accepts any; tests rely on that.Apply this diff:
-function tryParseJson(value: string): any; +function tryParseJson(value: any): any;
206-208: Duplicate/stray “## API” header.
Remove or rename; it breaks doc structure.Apply this diff:
-## API -
262-268: Clarify parsing scope to avoid surprising coercions.
If adopting the code change below to avoid parsing primitives, reflect it here.Apply this diff:
1. **JSON 解析优先级**:先尝试直接解析,失败后尝试 URL 解码再解析 +2. **解析范围**:仅当字符串看起来是对象或数组(以 `{}` 或 `[]` 包裹)时才解析;数值/布尔/`null` 等原始 JSON 不会被强制转换 2. **类型安全**:建议配合 TypeScript 使用以获得更好的类型提示 3. **性能考虑**:JSON 解析在 `onLoad` 生命周期中执行,不会影响页面性能 4. **错误处理**:解析失败时会保持原始字符串值,不会抛出错误 5. **响应式更新**:参数值在页面生命周期内保持响应式 6. **编码建议**:推荐使用 `encodeURIComponent` 来确保特殊字符的正确传输src/useQuery/index.test.ts (2)
10-47: Broaden useQuery tests to cover reactive key and value resolution.
Currently only shape is asserted. Add integration-like checks by mutatingquery.value.Apply this diff to add tests:
it('should work with key parameter', () => { const result = useQuery('testKey'); expect(result).toHaveProperty('query'); expect(result).toHaveProperty('value'); }); + it('should resolve value for given key and react to key changes', () => { + const key = ref<'id' | 'name'>('id'); + const { query, value } = useQuery(key); + // simulate onLoad result + query.value = { id: '1', name: 'Alice' }; + expect(value.value).toBe('1'); + key.value = 'name'; + expect(value.value).toBe('Alice'); + }); + + it('should treat empty-string key as valid', () => { + const key = ref(''); + const { query, value } = useQuery(key); + query.value = { '': 'empty', a: 1 }; + expect(value.value).toBe('empty'); + });
49-122: Tighten/extend parsing tests to cover edge cases and avoid primitive coercion.
Add cases for encoded whitespace and primitive JSON strings.Apply this diff to add tests:
describe('json parsing logic', () => { @@ it('should parse URL encoded JSON', () => { @@ }); + it('should parse URL encoded JSON with leading/trailing spaces after decode', () => { + const encodedWithSpaces = '%20%7B%22id%22%3A123%7D%20'; // ' {"id":123} ' + const result = tryParseJson(encodedWithSpaces); + expect(result).toEqual({ id: 123 }); + }); + + it('should not coerce primitive JSON strings (number/boolean/null)', () => { + expect(tryParseJson('123')).toBe('123'); + expect(tryParseJson('true')).toBe('true'); + expect(tryParseJson('null')).toBe('null'); + });src/useQuery/index.ts (1)
62-76: Avoid aliasing raw params when parseJson is false.
Return a new object to prevent accidental external mutation.Apply this diff:
const { parseJson = true } = options; if (!parseJson) { - return params; + return { ...params }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/exports.ts(1 hunks)src/index.md(1 hunks)src/useQuery/index.md(1 hunks)src/useQuery/index.test.ts(1 hunks)src/useQuery/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/useQuery/index.test.ts (1)
src/useQuery/index.ts (2)
useQuery(85-96)tryParseJson(20-54)
src/useQuery/index.ts (2)
src/types.ts (1)
MaybeRefOrGetter(7-7)src/tryOnLoad/index.ts (1)
tryOnLoad(15-47)
🔇 Additional comments (2)
src/index.md (1)
28-28: Docs nav entry for useQuery — LGTM.
Placement matches exports order.src/exports.ts (1)
26-26: Public export added — LGTM.
Matches docs and tests.
|
谢谢。 不过能否移除 因为 |
|
谢谢您的审查,我调整了代码,麻烦您再审核一下 |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/useQuery/index.ts (1)
1-2: Lock Vue ≥ 3.3 or remove 3.3-only APIs (toValue/MaybeRefOrGetter).You’re importing
toValueandMaybeRefOrGetterfromvue, which require Vue 3.3+. If the repo still supports 3.2, this will break. Choose one:
- Option A (preferred): bump peer/dev deps to Vue ^3.3.0 and note it in docs.
- Option B: keep Vue 3.2 compat by replacing
toValuewithunref+ function-call fallback and using the localMaybeRefOrGettertype.Run to verify declared Vue ranges:
#!/bin/bash fd package.json | while read -r f; do echo "== $f =="; rg -nE '"vue"\s*:\s*"[^\"]+"' "$f" -n || true rg -nE '"peerDependencies"\s*:\s*\{' -n -C3 "$f" || true doneIf you pick Option B, apply:
- import type { MaybeRefOrGetter } from 'vue'; - import { computed, ref, toValue } from 'vue'; + import type { MaybeRefOrGetter } from '../types'; + import { computed, ref, unref } from 'vue'; @@ - const value = computed(() => (key != null ? query.value[toValue(key)] : null)); + const value = computed(() => ( + key != null + ? query.value[typeof key === 'function' ? (key as () => string)() : unref(key)] + : null + ));Also applies to: 95-97
🧹 Nitpick comments (2)
src/useQuery/index.ts (2)
67-69: Avoid aliasing the original params object when parseJson is false.Returning
paramsby reference may couplequery.valueto the framework-provided object. Shallow‑clone to decouple.Apply:
- if (!parseJson) { - return params; - } + if (!parseJson) { + return { ...params }; + }
87-98: Add TS overloads to improvevaluetyping.Overloads make
valueComputedRef<null>when no key is passed andComputedRef<T | undefined>when a key is provided.Add before the implementation:
// Overloads export function useQuery(): { query: ReturnType<typeof ref<Record<string, any>>>; value: ReturnType<typeof computed<null>>; }; export function useQuery<T = any>(key: MaybeRefOrGetter<string>, options?: UseQueryOptions): { query: ReturnType<typeof ref<Record<string, any>>>; value: ReturnType<typeof computed<T | undefined>>; };Keep the current implementation as-is; TS will resolve to the proper signature.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/useQuery/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/useQuery/index.ts (2)
src/types.ts (1)
MaybeRefOrGetter(7-7)src/tryOnLoad/index.ts (1)
tryOnLoad(15-47)
🔇 Additional comments (1)
src/useQuery/index.ts (1)
18-56: tryParseJson logic looks solid and safe.Only parses object/array JSON; handles “+ as space”; decodes, trims, and falls back cleanly. Matches earlier feedback.
|
谢谢你的PR |
Summary by CodeRabbit