Conversation
|
Preview (prod) → https://189-prod.rucq-ui-preview.trapti.tech/ |
There was a problem hiding this comment.
Pull request overview
This PR enhances TypeScript configuration with stricter type checking options to improve code safety. The key change is enabling noUncheckedIndexedAccess to handle array element access that may return undefined, along with other strict compiler options. The PR also updates TypeScript and typescript-eslint dependencies and adjusts ESLint rules for better unused variable detection.
Changes:
- Enhanced tsconfig.app.json with stricter type checking options (noUncheckedIndexedAccess, noUnusedLocals, strict mode)
- Updated TypeScript version and added typescript-eslint package
- Configured ESLint rule for unused variables with underscore prefix support
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.app.json | Added strict type checking options including noUncheckedIndexedAccess, changed lib to ESNext, added vite/client types, removed env.d.ts from includes |
| package.json | Downgraded TypeScript to 5.9.3, added typescript-eslint 8.52.0 |
| package-lock.json | Updated dependency versions to match package.json changes |
| eslint.config.js | Added @typescript-eslint/no-unused-vars rule configuration with underscore prefix support |
| env.d.ts | Removed file containing custom environment variable type definitions |
| .vscode/settings.json | Added TypeScript SDK path configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "types": ["vite/client"], | ||
| // 環境変数 import.meta.env や静的アセットの型定義を宣言 |
There was a problem hiding this comment.
The removal of env.d.ts (which previously defined ImportMetaEnv with VITE_CHANNEL_PATH) means custom environment variables are no longer typed. While "types": ["vite/client"] provides base import.meta.env types, it doesn't include project-specific environment variables like VITE_CHANNEL_PATH (used in src/views/RollCallView.vue). Consider creating a vite-env.d.ts or similar file to declare these custom types, or restore the env.d.ts file with its type definitions.
There was a problem hiding this comment.
おっしゃる通り import.meta.env.VITE_CHANNEL_PATH はこの変更に伴い string 型から any 型になりました。好ましくない変化ではありますが致し方ないように思います
環境変数の例を .env などにかき、その型を env.d.ts にかくという運用は保守性に難があります。必要な環境変数が増えたとき、.env の更新自体はされても env.d.ts の型の更新は忘れられがちになるでしょうから、いっそ env.d.ts ごと消して any 型を前提とした運用にするのがよいと考えました。既存の VITE_CHANNEL_PATH の出現箇所は any のままでもエラー等を生じていないようです。この先環境変数が増えたら zod などのライブラリを利用することを視野に入れていますが、今はほとんど VITE_CHANNEL_PATH だけなのでそこまでしなくてもよいかと思っています
|
|
||
| "strict": true, | ||
| "noUnusedLocals": true, | ||
| "erasableSyntaxOnly": true, |
There was a problem hiding this comment.
The erasableSyntaxOnly compiler option is not a valid TypeScript compiler option. This option does not exist in the TypeScript compiler. You may have meant a different option, or this should be removed. Valid related options include isolatedModules or verbatimModuleSyntax.
| "erasableSyntaxOnly": true, |
|
いくつかのリファクタリングを行いビルドを通しました |
📝 WalkthroughWalkthroughTypeScript 設定を厳格化し、ESLint ルールと依存を更新。複数コンポーネント／ユーティリティに非 null アサーションや防御的初期化を追加し、環境型定義を削除、VSCode でローカル TypeScript を使う設定を追加しました。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/event/utils/eventGrid.ts (1)
66-73:⚠️ Potential issue | 🔴 Critical
previousが未定義です - ランタイムエラーが発生しますLine 68 で
previous.timeを参照していますが、previousはこのスコープ（1つ目のループ）では定義されていません。previousは Line 82 の2つ目のループでのみ定義されています。この条件分岐に入ると
ReferenceError: previous is not definedが発生します。🐛 修正案
if (this.rows[i - 1].events.length === 1 && this.rows[i].events.length === 0) { this.rows.splice(i, 0, { - time: new Date(previous.time), + time: new Date(this.rows[i - 1]!.time), events: [], minHeight: 'narrow', stampAlign: 'none', }) }
🤖 Fix all issues with AI agents
In `@src/components/event/EventEditor.vue`:
- Line 114: The current initialization sets color.value =
EVENT_COLORS[Math.floor(Math.random() * EVENT_COLORS.length)]! which can yield
undefined if EVENT_COLORS is empty; update the logic around EVENT_COLORS and
color.value (the random color assignment) to guard against an empty array by
providing a fallback (e.g., a default color constant) or an explicit check that
sets color.value to the fallback when EVENT_COLORS.length === 0, ensuring no
non-null assertion is relied upon.
In `@src/components/information/AnswersDialog.vue`:
- Line 31:
byAnswer[...]アクセスで非null断言(!)して直接pushしているため、キーが未初期化だとクラッシュします。AnswersDialog.vue内の
byAnswer[answer.selectedOption.content]!.push(answer.userId) と同様に使っている箇所（該当は
lines 31, 40, 49, 57 相当）で、push の前に byAnswer[answer.selectedOption.content] ??=
[] を使って配列を確実に初期化してから answer.userId を push するように修正してください；対象となる識別子は
byAnswer、answer.selectedOption.content、answer.userId です。
🧹 Nitpick comments (1)
src/components/information/QuestionGroupEditor.vue (1)
41-41: QuestionGroupViewer.vue との一貫性を保つため、オプショナルチェーンを使用してください。
answersMapは親コンポーネント (QuestionGroupPanel) で全問題について初期化されるため、現在のコードは技術的には安全です。ただし、読み取り専用の QuestionGroupViewer.vue ではanswersMap[question.id]?.valueとオプショナルチェーンが使われており、エディタコンポーネントでも一貫性を持たせることが望ましいです。非null アサーション!をオプショナルチェーン?.に変更することで、より防御的で堅牢なコードになります。推奨される変更
- get: () => props.answersMap[questionId]!.value, + get: () => props.answersMap[questionId]?.value,
| } else { | ||
| organizerId.value = userStore.user.id | ||
| color.value = EVENT_COLORS[Math.floor(Math.random() * EVENT_COLORS.length)] // 色をランダムで初期化 | ||
| color.value = EVENT_COLORS[Math.floor(Math.random() * EVENT_COLORS.length)]! // 色をランダムで初期化 |
There was a problem hiding this comment.
EVENT_COLORS が空のときに undefined になります。
非nullアサーションで型は通りますが、配列が空なら color が undefined になり得ます。最低限フォールバックか明示的なガードを入れておくと安心です。
✅ 例: フォールバックを追加
- color.value = EVENT_COLORS[Math.floor(Math.random() * EVENT_COLORS.length)]! // 色をランダムで初期化
+ const randomColor = EVENT_COLORS[Math.floor(Math.random() * EVENT_COLORS.length)]
+ color.value = randomColor ?? color.value // 空配列時は既定値を維持📝 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.
| color.value = EVENT_COLORS[Math.floor(Math.random() * EVENT_COLORS.length)]! // 色をランダムで初期化 | |
| const randomColor = EVENT_COLORS[Math.floor(Math.random() * EVENT_COLORS.length)] | |
| color.value = randomColor ?? color.value // 空配列時は既定値を維持 |
🤖 Prompt for AI Agents
In `@src/components/event/EventEditor.vue` at line 114, The current initialization
sets color.value = EVENT_COLORS[Math.floor(Math.random() *
EVENT_COLORS.length)]! which can yield undefined if EVENT_COLORS is empty;
update the logic around EVENT_COLORS and color.value (the random color
assignment) to guard against an empty array by providing a fallback (e.g., a
default color constant) or an explicit check that sets color.value to the
fallback when EVENT_COLORS.length === 0, ensuring no non-null assertion is
relied upon.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/event/utils/eventGrid.ts`:
- Around line 113-116: In addMomentEvent, guard against getIndex returning -1
before indexing this.rows: assign const idx =
this.getIndex(getStartTime(event)); if idx is -1 either create/append a new row
at the appropriate position or early-return (choose consistent behavior with
addDurationEvent) instead of doing this.rows[idx]!, and then use this.rows[idx]
to push the moment event; update any downstream code that assumes a row exists
so you don't dereference undefined.
|
コンフリクトが生じていたので main のマージと、ついでに安全処理を行いました |
#188 の作業中に本来エラーになるべき場所がエラーになっていないような感じがしたので、tsconfig.app.json にいくつかの設定を加えてさらに型チェックを厳しくすることを提案します
核となる制限は
"noUncheckedIndexedAccess": trueであり、配列外参照が undefined となる仕様を型解釈に取り入れます追記：このままだとビルドが落ちていてずっと main にマージすることができないので、
#188 がマージされ次第 main をマージしてきて必要なコードの修正を行うことによりビルドを通す予定です
Summary by CodeRabbit
リリースノート
チョア
リファクター
破壊的変更