feat: openapi vue query - #110
Conversation
WalkthroughOpenAPI 型に基づく型付き TanStack Vue Query クライアントを追加し、対応するテストを導入、ラベル一覧ページをルートパラメータ（tenant/projectKey）駆動でテナント別エンドポイントへ切替えて表示を調整します。 ChangesVue Query API クライアント統合
Sequence Diagram(s)sequenceDiagram
participant Client as ラベルページ
participant Router as usePageContext
participant ApiClient as apiClient
participant Server as Backend API
Client->>Router: routeParams 取得
Router-->>Client: tenant, projectKey
Client->>Client: パラメータ型チェック
alt 有効
Client->>ApiClient: projectLabelsQueryOptions(tenant, projectKey)
ApiClient->>Server: GET /v1/tenants/{tenant_id}/projects/{project_id}/labels
Server-->>ApiClient: labels 配列
ApiClient-->>Client: labels 状態更新
Client->>Client: ラベルリスト表示
else 無効
Client->>Client: エラーメッセージ設定
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/frontend/package.json`:
- Line 20: The dependency on the local tarball for `@koyori-app/openapi-vue-query`
(currently "file:/tmp/koyori-0.1.3.tgz") makes installs unreproducible; change
the dependency in apps/frontend package.json to a published semver (e.g.
"0.1.3") or to a workspace reference (e.g. "workspace:^0.1.3" or the local
workspace spec) so it no longer points to /tmp, then run your package manager
(pnpm install) to regenerate pnpm-lock.yaml and verify the lockfile no longer
references /tmp/koyori-0.1.3.tgz; also ensure the package is listed in the repo
workspaces if you choose the workspace option.
In `@apps/frontend/src/pages/`@tenant/projects/@projectKey/labels/+Page.vue:
- Around line 19-29: The code currently reads projectKey from
pageContext.routeParams and passes it directly as the OpenAPI path param
project_id to api.GET, but projectKey and project_id are distinct concepts so
this causes wrong/empty responses; fix by either (A) making the route supply the
actual project_id (rename/align the route param so
pageContext.routeParams.project_id is present and used) or (B) resolve
projectKey → project_id before calling api.GET (e.g., call the project lookup
endpoint using projectKey, extract the real id, then pass that id into api.GET);
update the code paths that reference projectKey, pageContext.routeParams, and
the api.GET call to use the resolved project_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7bfce99b-d7b1-4aca-a87c-556b41050997
⛔ Files ignored due to path filters (1)
apps/frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.gitignoreapps/frontend/package.jsonapps/frontend/pnpm-workspace.yamlapps/frontend/src/lib/__tests__/api-vue-query.test.tsapps/frontend/src/lib/api-vue-query.tsapps/frontend/src/pages/@tenant/projects/@projectKey/labels/+Page.vue
…arams Rename local binding projectKey → projectId to match the project_id param the API expects. Adds a comment noting the key=id assumption; if slug vs UUID semantics diverge later, resolve before the API call at that point. Addresses CodeRabbit finding on PR #110. Assisted-by: multi-agent-shogun-aki-tweak
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/pages/@tenant/projects/@projectKey/labels/+Page.vue (1)
28-36: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winVue Query 統合の利用を検討してください。
このコンポーネントは
useDefaultApi()(生の openapi-fetch クライアント) を使用していますが、本 PR で追加されたapi-vue-query.tsにはprojectLabelsQueryOptionsヘルパーが用意されています。Vue Query を使うことでキャッシュ、自動再フェッチ、devtools 連携などの恩恵を受けられます。import { useQuery } from '`@tanstack/vue-query`'; import { projectLabelsQueryOptions } from '`@/lib/api-vue-query`'; // onMounted の代わりに useQuery を使用 const { data: labels, isLoading: loading, error } = useQuery( projectLabelsQueryOptions(tenant, projectId) );段階的な移行を意図している場合は、現状のままでも問題ありません。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/frontend/src/pages/`@tenant/projects/@projectKey/labels/+Page.vue around lines 28 - 36, The component is using the raw openapi-fetch client via useDefaultApi() and manual await/if logic; switch to the provided Vue Query helper by importing projectLabelsQueryOptions from api-vue-query.ts and using useQuery(projectLabelsQueryOptions(tenant, projectId)) instead of the manual GET call in onMounted; replace labels, error and loading state with the values returned from useQuery (data as labels, isLoading as loading, error) and remove the manual fetch/error assignment and the useDefaultApi() GET invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/frontend/src/pages/`@tenant/projects/@projectKey/labels/+Page.vue:
- Around line 28-36: The component is using the raw openapi-fetch client via
useDefaultApi() and manual await/if logic; switch to the provided Vue Query
helper by importing projectLabelsQueryOptions from api-vue-query.ts and using
useQuery(projectLabelsQueryOptions(tenant, projectId)) instead of the manual GET
call in onMounted; replace labels, error and loading state with the values
returned from useQuery (data as labels, isLoading as loading, error) and remove
the manual fetch/error assignment and the useDefaultApi() GET invocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e45da775-29cf-4dde-83b5-a34e9f49fbd7
📒 Files selected for processing (1)
apps/frontend/src/pages/@tenant/projects/@projectKey/labels/+Page.vue
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/frontend/src/pages/`@tenant/projects/@projectKey/labels/+Page.vue:
- Around line 31-34: The error text is incorrect for the projects list fetch:
update the string assigned to error.value in the projectsError handler (the
block that checks projectsError) from 'Failed to fetch project' to 'Failed to
fetch projects' so the message accurately reflects that multiple projects were
being fetched.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bcb21098-16e1-48ba-a06e-1c16cb90e481
📒 Files selected for processing (1)
apps/frontend/src/pages/@tenant/projects/@projectKey/labels/+Page.vue
Add typed api-vue-query client, unit tests with mocked fetch, and fix labels page OpenAPI path so vue-tsc passes on the task schema. Assisted-by: multi-agent-shogun-aki-tweak
…ue-query 0.1.3 - Remove PathsWithMethod workaround types (fixed upstream in 0.1.2) - Remove manual type casts; useQuery/useMutation now fully infer return types - Rename createTestApiVueQueryClient → createTestApiClient; expose raw client - Update tests: withQuery<T> helper for zero-annotation inference, add error and queryOptions integration cases (6 tests total) - Bump to 0.1.3: exports UseQueryResult/UseMutationResult<CreatedClient, ...> Assisted-by: multi-agent-shogun-aki-tweak
Remove queryOptions+useQuery integration test — QueryOptionsFunction return type carries [key: string]: any which is incompatible with @tanstack/vue-query useQuery overloads under vue-tsc strict checking. Behavior is already covered by the withQuery helper tests. Assisted-by: multi-agent-shogun-aki-tweak
…and 0.1.2 Assisted-by: multi-agent-shogun-aki-tweak
…arams Rename local binding projectKey → projectId to match the project_id param the API expects. Adds a comment noting the key=id assumption; if slug vs UUID semantics diverge later, resolve before the API call at that point. Addresses CodeRabbit finding on PR #110. Assisted-by: multi-agent-shogun-aki-tweak
8f39fae to
2fe7c57
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/frontend/src/lib/__tests__/api-vue-query.test.ts`:
- Around line 60-62: The test suite is not unmounting components after each
test, causing test contamination. Currently the afterEach hook only calls
queryClient.clear() but does not clean up mounted Vue components. Store a
reference to the wrapper returned by the mount() call in the test cases (around
lines 64-80), and update the afterEach hook to call wrapper.unmount() after
clearing the queryClient to ensure all mounted components, their watchers, and
subscriptions are properly cleaned up between tests.
- Around line 86-89: Single `flushPromises()` calls can miss Vue Query state
transitions and cause test instability. Replace the `flushPromises()` call at
line 86-89 (before checking `query.isSuccess.value`) with `vi.waitFor(() =>
expect(query.isSuccess.value).toBe(true))` to ensure the state fully transitions
before assertions. Apply the same fix at the sibling location (lines 125-127) by
replacing the single `flushPromises()` call with `vi.waitFor` to conditionally
wait for the relevant state condition (either `isSuccess` or `isError`) before
verifying the query state and data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3c9a6429-9811-4c6a-b469-03153f349c30
⛔ Files ignored due to path filters (1)
apps/frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.gitignoreapps/frontend/package.jsonapps/frontend/pnpm-workspace.yamlapps/frontend/src/lib/__tests__/api-vue-query.test.tsapps/frontend/src/lib/api-vue-query.tsapps/frontend/src/pages/@tenant/projects/@projectKey/labels/+Page.vue
| afterEach(() => { | ||
| queryClient.clear(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/frontend/src/lib/__tests__/api-vue-query.test.ts | head -100Repository: koyori-app/task
Length of output: 3450
マウントしたコンポーネントを未解放のままにしており、テスト間汚染を招きます。
Line 64-80 で mount() した wrapper を参照保持していない上、Line 60-62 の afterEach では queryClient.clear() のみです。複数のテストが実行される際、各 withQuery() の呼び出しで新たなコンポーネントが mount されますが、一度も unmount されません。これにより watchers や subscriptions がテスト間で残り、テストの flakiness や汚染を招きます。各テスト後に確実に unmount してください。
🔧 修正例
-import { mount, flushPromises } from '`@vue/test-utils`';
+import { mount, flushPromises, type VueWrapper } from '`@vue/test-utils`';
@@
describe('api-vue-query PoC', () => {
+ const wrappers: VueWrapper[] = [];
@@
afterEach(() => {
+ for (const wrapper of wrappers) wrapper.unmount();
+ wrappers.length = 0;
queryClient.clear();
});
@@
function withQuery<T>(setup: () => T): T {
let result!: T;
- mount(
+ const wrapper = mount(
defineComponent({
@@
},
);
+ wrappers.push(wrapper);
return result;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/frontend/src/lib/__tests__/api-vue-query.test.ts` around lines 60 - 62,
The test suite is not unmounting components after each test, causing test
contamination. Currently the afterEach hook only calls queryClient.clear() but
does not clean up mounted Vue components. Store a reference to the wrapper
returned by the mount() call in the test cases (around lines 64-80), and update
the afterEach hook to call wrapper.unmount() after clearing the queryClient to
ensure all mounted components, their watchers, and subscriptions are properly
cleaned up between tests.
| await flushPromises(); | ||
|
|
||
| expect(query.isSuccess.value).toBe(true); | ||
| expect(query.data.value).toEqual(mockUser); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/frontend/src/lib/__tests__/api-vue-query.test.ts | head -150Repository: koyori-app/task
Length of output: 5098
🏁 Script executed:
find . -name "vitest.config.*" -o -name "package.json" | head -5Repository: koyori-app/task
Length of output: 112
🏁 Script executed:
cat apps/frontend/package.jsonRepository: koyori-app/task
Length of output: 2479
🏁 Script executed:
find . -name "vitest.config.*" -o -name "vite.config.*" | grep -E "(vitest|vite)\.config"Repository: koyori-app/task
Length of output: 116
🏁 Script executed:
cat apps/frontend/vite.config.tsRepository: koyori-app/task
Length of output: 3992
🏁 Script executed:
cat -n apps/frontend/pnpm-lock.yaml 2>/dev/null | grep -A 5 '"vitest"' | head -20Repository: koyori-app/task
Length of output: 41
🌐 Web query:
Vue Query vitest testing flushPromises vs vi.waitFor best practices
💡 Result:
When testing Vue Query with Vitest, the choice between flushPromises, vi.waitFor, and vi.advanceTimersByTimeAsync depends on what you are trying to achieve [1][2]. Best practices for TanStack Query testing in Vitest are summarized below: 1. Use vi.advanceTimersByTimeAsync for Time-Based Operations If your tests utilize Vitest fake timers (vi.useFakeTimers), explicitly advancing timers is the most reliable and performant way to test queries [1]. Instead of using polling-based waits, you can advance time directly to resolve internal query timeouts or delays, which reduces test execution time and avoids flakiness [1]. 2. Use vi.waitFor for Polling Assertions vi.waitFor is best suited for scenarios where you need to poll for a specific condition to become true, such as waiting for a reactive state (e.g., isSuccess) to change [3][4]. It is highly effective when the exact timing of the state change is not immediately controllable or when testing asynchronous side effects that settle over an indeterminate duration [5][3]. 3. Use flushPromises for Resolving Pending Promises flushPromises (from @vue/test-utils) is useful for ensuring all currently pending microtasks (promises) are resolved [6]. It is frequently used to clear API call queues when you are mocking requests and want to immediately progress the test to the data-loaded state [2][6]. Note that in some complex setups (e.g., using MSW), you may need to call flushPromises multiple times to resolve chained promises [2][7]. Summary of Selection Criteria: - Use vi.advanceTimersByTimeAsync when fake timers are enabled to precisely trigger query updates [1][8]. - Use vi.waitFor when you need to assert against a state that will eventually become true [3][4]. - Use flushPromises to force the resolution of all pending promises when simple synchronization is needed [2][6]. TanStack Query official testing documentation often utilizes vi.waitFor for hooks testing to ensure the query has reached the expected status [3][4], while internal library tests increasingly favor vi.advanceTimersByTimeAsync for performance and reliability when fake timers are active [1].
Citations:
- 1: test(query-core/hydration): replace 'vi.waitFor' with 'vi.advanceTimersByTimeAsync' TanStack/query#10428
- 2: https://github.com/antfu/skills/blob/main/skills/vue-testing-best-practices/reference/testing-async-await-flushpromises.md
- 3: https://tanstack.com/query/latest/docs/framework/react/guides/testing
- 4: https://tanstack.dev/query/v5/docs/framework/react/guides/testing
- 5: Helpers: nextTick/flushPromises vuejs/test-utils#137
- 6: https://test-utils.vuejs.org/guide/advanced/async-suspense.html
- 7: Helpers: nextTick/flushPromises vuejs/test-utils#137
- 8: Query never updates when using Vitest fake timers TanStack/query#6994
🌐 Web query:
vitest vi.waitFor API documentation when was it introduced
💡 Result:
The vi.waitFor API in Vitest was introduced in version 0.34.5 [1][2][3]. It was added as a new feature to support waiting for asynchronous actions or conditions to be met during tests [4][5]. The documentation for vi.waitFor explicitly notes this version as the origin [1][2][6].
Citations:
- 1: https://www.getbook.com/en/book/vitest-0/api/vi
- 2: https://v0.vitest.dev/api/vi
- 3: https://www.getbook.com/en/book/vitest-1/api/vi
- 4: feat(vitest): support
vi.waitFormethod vitest-dev/vitest#4113 - 5: feat(vitest): support
vi.waitFormethod vitest-dev/vitest#4113 - 6: vitest-dev/vitest@v0.34.5...v0.34.6
flushPromises() 1回の待機では Vue Query の状態遷移を取りこぼす可能性があります。
Line 86 と Line 125 の単発 flushPromises() 依存はテストの不安定性につながりやすいです。isSuccess / isError などの状態確認には、条件待機 (vi.waitFor) を使用してください。TanStack Query の公式テスティングドキュメントでも、フック検証時には vi.waitFor が推奨されています。
🔧 修正例
- await flushPromises();
-
- expect(query.isSuccess.value).toBe(true);
+ await vi.waitFor(() => {
+ expect(query.isSuccess.value).toBe(true);
+ });
expect(query.data.value).toEqual(mockUser);
@@
- await flushPromises();
-
- expect(query.isError.value).toBe(true);
+ await vi.waitFor(() => {
+ expect(query.isError.value).toBe(true);
+ });Also applies to: 125-127
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/frontend/src/lib/__tests__/api-vue-query.test.ts` around lines 86 - 89,
Single `flushPromises()` calls can miss Vue Query state transitions and cause
test instability. Replace the `flushPromises()` call at line 86-89 (before
checking `query.isSuccess.value`) with `vi.waitFor(() =>
expect(query.isSuccess.value).toBe(true))` to ensure the state fully transitions
before assertions. Apply the same fix at the sibling location (lines 125-127) by
replacing the single `flushPromises()` call with `vi.waitFor` to conditionally
wait for the relevant state condition (either `isSuccess` or `isError`) before
verifying the query state and data.
2 participants
これを元にラッパー構築。
#8 のPhase 0.5
related to #104
resolve #112
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests
Chores