feat: setup pinia - #114
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Walkthroughフロントエンドに Pinia(v3.0.4)および ChangesPinia 統合と Auth ストア
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 |
0b3ad3b to
4af18e0
Compare
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/stores/auth.ts`:
- Around line 4-11: The AuthUser interface in the auth store contains sensitive
fields (email, is_admin, is_suspended, totp_enabled) that are being persisted to
localStorage unencrypted due to the `persist: true` configuration. To fix this
security issue, locate the pinia store configuration for the auth store and use
the `pick` option from pinia-plugin-persistedstate to restrict persistence to
only the minimum necessary fields for display (such as id and username),
explicitly excluding all sensitive fields like email, is_admin, is_suspended,
and totp_enabled. This ensures that sensitive user information is not stored in
plain text on the client side.
🪄 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: 12094683-d6e6-4ed3-987c-3d4848b56e50
⛔ Files ignored due to path filters (1)
apps/frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/frontend/package.jsonapps/frontend/src/pages/+config.tsapps/frontend/src/pages/+onCreatePinia.tsapps/frontend/src/stores/auth.ts
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/stores/auth.ts (1)
17-17:⚠️ Potential issue | 🟡 Minor
userの型定義と永続化フィールドの不整合を改善してください。
AuthUserは 7 つの必須フィールドで定義されていますが、永続化設定は['user.id', 'user.username', 'user.email']のみです。ページリロード時、復元されたuserオブジェクトはemail_verified、is_admin、is_suspended、totp_enabledを欠いた部分型となり、型システムの契約を破ります。現在のコードではこれらのフィールドへのアクセスが見つかりませんが、今後の機能追加時に
undefinedを前提としないアクセスが発生する可能性があります。永続化対象を明示的に分離するか、復元後のuserを部分型として型安全に扱う設計に改善してください。🤖 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/stores/auth.ts` at line 17, The `user` ref in the auth.ts file is typed as `AuthUser | null`, but the persistence configuration only saves three fields (id, username, email), causing a type mismatch when the page reloads. The restored user object will lack the four required fields (email_verified, is_admin, is_suspended, totp_enabled), breaking the type contract. To fix this, either create a separate partial type (e.g., `PartialAuthUser`) that reflects only the persisted fields and use it for the `user` ref type annotation, or update the persistence configuration to include all seven required fields of `AuthUser` to ensure the restored object matches the full type definition. Choose the approach that best aligns with your application's requirements for which user data should persist across page reloads.
🤖 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/stores/auth.ts`:
- Line 17: The `user` ref in the auth.ts file is typed as `AuthUser | null`, but
the persistence configuration only saves three fields (id, username, email),
causing a type mismatch when the page reloads. The restored user object will
lack the four required fields (email_verified, is_admin, is_suspended,
totp_enabled), breaking the type contract. To fix this, either create a separate
partial type (e.g., `PartialAuthUser`) that reflects only the persisted fields
and use it for the `user` ref type annotation, or update the persistence
configuration to include all seven required fields of `AuthUser` to ensure the
restored object matches the full type definition. Choose the approach that best
aligns with your application's requirements for which user data should persist
across page reloads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4665d267-0ba8-435c-8c29-255d87c9765a
📒 Files selected for processing (1)
apps/frontend/src/stores/auth.ts
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/stores/auth.ts`:
- Line 29: The `persist.pick` configuration on Line 29 only persists three user
fields (id, username, email), but the `AuthUser` type on Line 17 declares all
fields as required. After a page reload, unpersisted fields like email_verified,
is_admin, is_suspended, and totp_enabled will be undefined, creating a
type/runtime mismatch. Fix this by making the unpersisted fields optional in the
AuthUser type definition by adding the optional marker (?) to fields that are
not included in the persist.pick array. This ensures the type accurately
reflects what data will be available after rehydration.
🪄 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: d7711e7e-3d3d-4593-b916-e48ad8a576b3
📒 Files selected for processing (1)
apps/frontend/src/stores/auth.ts
Add pinia and vike-vue-pinia packages, register vikeVuePinia in +config.ts extends array. Enables automatic Pinia store creation and SSR state hydration via the vike-vue extension. Closes #113 Assisted-by: multi-agent-shogun-aki-tweak
- Install pinia-plugin-persistedstate - Register via +onCreatePinia hook (client-only, sessionStorage) - Add useAuthStore with AuthUser type and setUser/clearUser actions Assisted-by: multi-agent-shogun-aki-tweak
…ionStorage sessionStorage clears on tab close causing UI flicker on reopen. localStorage is appropriate since only display-safe user info is cached (no tokens — session auth is handled by httpOnly cookie). Assisted-by: multi-agent-shogun-aki-tweak
…me only Persisting sensitive fields (email, is_admin, is_suspended, totp_enabled) to localStorage unencrypted poses a security risk. Use pick option from pinia-plugin-persistedstate to limit persistence to display-only fields. Assisted-by: multi-agent-shogun-aki-tweak
…splay email is shown in NavUser and must survive page reload without re-fetching. Only auth-decision fields (is_admin, is_suspended, totp_enabled) remain excluded. Assisted-by: multi-agent-shogun-aki-tweak
b158e95 to
3e9712e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/frontend/src/stores/auth.ts (1)
4-12:⚠️ Potential issue | 🟡 Minor
AuthUser型と永続化フィールドの不整合が残っています(過去コメントの重複)。Line 29 の
persist.pickはuser.id、user.username、user.emailの 3 フィールドのみを永続化しますが、Line 17 のuser型は全フィールド必須のAuthUserとして宣言されています。ページ再読み込み後、email_verified、is_admin、is_suspended、totp_enabledはundefinedになりますが、TypeScript は存在すると判断します。過去のレビューコメントで既に指摘されていますが、未解決のままです。永続化されないフィールドを
AuthUserインターフェースで optional (?) にするか、または再読み込み後に API から完全なユーザーデータを再取得する設計であることを明示的にドキュメント化してください。Also applies to: 17-17, 29-29
🤖 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/stores/auth.ts` around lines 4 - 12, The AuthUser interface at lines 4-12 declares all fields as required, but the persist.pick call at line 29 only persists three fields (user.id, user.username, user.email), causing the other fields (email_verified, is_admin, is_suspended, totp_enabled) to become undefined after page reload while TypeScript incorrectly assumes they exist. Fix this mismatch by either making the non-persisted fields optional with the `?` modifier in the AuthUser interface definition at lines 4-12, or by adding a clear comment at line 29 documenting that the design expects to re-fetch complete user data from the API after a reload, and update the user type declaration at line 17 to reflect this pattern accordingly.
🤖 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/`+onCreatePinia.ts:
- Around line 8-10: Remove the unsafe `as any` type assertion from `pageContext`
in the pinia initialization block. Instead of casting the entire `pageContext`
object to `any`, properly type the `pageContext` parameter to include the
`pinia` property. This can be done by importing the appropriate type from
vike-vue-pinia or defining the correct type for `PageContext` that includes
proper type information for the `pinia` property, so that the
`pageContext.pinia?.use(...)` call maintains full type safety without needing
the `as any` assertion.
---
Duplicate comments:
In `@apps/frontend/src/stores/auth.ts`:
- Around line 4-12: The AuthUser interface at lines 4-12 declares all fields as
required, but the persist.pick call at line 29 only persists three fields
(user.id, user.username, user.email), causing the other fields (email_verified,
is_admin, is_suspended, totp_enabled) to become undefined after page reload
while TypeScript incorrectly assumes they exist. Fix this mismatch by either
making the non-persisted fields optional with the `?` modifier in the AuthUser
interface definition at lines 4-12, or by adding a clear comment at line 29
documenting that the design expects to re-fetch complete user data from the API
after a reload, and update the user type declaration at line 17 to reflect this
pattern accordingly.
🪄 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: 89db9cd1-d998-4861-93a2-b28ba1ddce42
⛔ Files ignored due to path filters (1)
apps/frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/frontend/package.jsonapps/frontend/src/pages/+config.tsapps/frontend/src/pages/+onCreatePinia.tsapps/frontend/src/stores/auth.ts
Assisted-by: multi-agent-shogun-aki-tweak
pinia導入
お試しのストアまで構築
blocking by #110 (#112)doneSummary by CodeRabbit
リリースノート