-
-
Notifications
You must be signed in to change notification settings - Fork 10
💥 Bump typescript from 5.6.3 to 5.8.3 (#2199) #2200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes update the TypeScript dependency from version 5.6.3 to 5.8.3 in the development environment. Additionally, several Svelte and TypeScript files are refactored to improve state management, user authentication checks, and data normalization, without altering public APIs or overall control flow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Server
participant AuthService
participant DB
User->>Server: Submit create workbook form
Server->>AuthService: getLoggedInUser(locals)
AuthService-->>Server: author or null
alt author is null
Server-->>User: Return 403, login required
else author exists
Server->>DB: Create workbook with form data (id: 0)
DB-->>Server: Workbook created
Server-->>User: Success response
end
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/routes/workbooks/[slug]/+page.svelte (1)
78-78: Prefer generic form for$stateto keep strict typing
let updatingModal: UpdatingModal | null = $state(null);widens the type toanybecause$state()returnsWritable<any>without a generic. Use the generic overload to preserve the union type:-let updatingModal: UpdatingModal | null = $state(null); +let updatingModal = $state<UpdatingModal | null>(null);This keeps TS strict-null checks helpful inside
handleClick.src/routes/workbooks/create/+page.server.ts (1)
62-67: No need to send a dummyid– Prisma will ignore unknown IDsPassing
id: 0is harmless but unnecessary noise. Omitting it simplifies the object and avoids confusion when reading logs.-const workBook = { - ...form.data, - id: 0, // Dummy id (Prisma will auto-generate it) -}; +const workBook = { ...form.data };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.json(1 hunks)src/routes/workbooks/[slug]/+page.svelte(1 hunks)src/routes/workbooks/create/+page.server.ts(3 hunks)src/routes/workbooks/edit/[slug]/+page.server.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: preview
🔇 Additional comments (2)
package.json (1)
67-67: Confirm tool-chain compatibility after jumping two minor TS versionsTypeScript 5.8 raises the minimum Node.js requirement to 16.14 and drops some deprecated compiler options. Double-check that CI / runtime images satisfy this and that
tsconfig.jsondoesn’t contain removed flags (e.g.preserveWatchOutput).Run:
#!/bin/bash # Ensure tsconfig has no removed/renamed options and Node version is high enough grep -E "\"preserveWatchOutput\"|\"incremental\"" -n tsconfig.json || true node -p "process.versions.node"src/routes/workbooks/edit/[slug]/+page.server.ts (1)
24-28: 👍 Defensive null-elimination forurlSluglooks goodExplicitly converting a possible
nulltoundefinedprevents superforms from emitting a nullable field. Nice catch.
KATO-Hiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
close #2199
Summary by CodeRabbit
Bug Fixes
urlSlug, preventing potential issues with form handling.Chores