V3/update deps #620
V3/update deps #620
Conversation
WalkthroughProject-wide migration of Zod field helpers to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as SearchInput
participant Evt as useEffectEvent(onChange)
participant App as Consumer
User->>UI: types characters
UI->>UI: update local state, restart debounce timer
Note right of UI #f0f9ff: debouncing via useEffectEvent\n(no ref indirection)
UI-->>Evt: debounce expires
Evt->>App: onChange(search)
App->>App: handle search (fetch/update)
sequenceDiagram
autonumber
actor Caller
participant AuthSvc as auth.databaseHooks
participant DB as Database
Caller->>AuthSvc: create/update user request
AuthSvc->>AuthSvc: if DEMO MODE -> throw
alt not demo
AuthSvc->>DB: persist user (return { data })
DB-->>AuthSvc: persisted user
AuthSvc-->>Caller: success
else demo
AuthSvc-->>Caller: throw DEMO MODE error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/server/routers/genre.ts (1)
37-37: Incorrect log message (“books” instead of “genres”)Small mismatch; adjust for clarity.
- context.logger.info('Getting books from database'); + context.logger.info('Getting genres from database');src/server/routers/book.ts (1)
211-215: Wrong permission scope for deleteById (likely security/authorization bug)Router is “book”, but permission checks “user: ['delete']”. This can allow unintended access or block legit access.
deleteById: protectedProcedure({ permission: { - user: ['delete'], + book: ['delete'], }, })
♻️ Duplicate comments (1)
src/routes/manager/users/index.tsx (1)
11-11: Verifyprefault()usage (see related comment in src/env/server.ts).This uses the same
prefault()method that needs verification. See the detailed comment insrc/env/server.tsfor the verification script.
🧹 Nitpick comments (7)
src/routes/login/verify.index.tsx (1)
10-13: Approvez.email()migration in verify.index.tsx
- Verified
zod@4.1.12and@tanstack/zod-adapter@1.132.47supportz.email().- Confirmed
PageLoginVerifystill expectsemail: string.- Optional: align with
zu.field*helpers for consistency across schemas.src/components/ui/search-input.tsx (1)
59-62: Consider usinguseEffectfor prop synchronization.The conditional
setSearchcall during render is valid for prop synchronization in React, but it's somewhat unconventional and can be harder to reason about. Consider refactoring to useuseEffectfor clarity:const externalValueHasChanged = useValueHasChanged(value); - if (externalValueHasChanged && value !== search) { - setSearch(value ?? ''); - } + + useEffect(() => { + if (externalValueHasChanged && value !== search) { + setSearch(value ?? ''); + } + }, [externalValueHasChanged, value, search]);This makes the synchronization logic more explicit and groups it with other effects. However, the current implementation is functionally correct.
src/hooks/use-value-has-changed.ts (1)
5-8: Consider usinguseEffectto avoid lint suppression.While mutating
valueRef.currentduring render is safe for this specific use case, consider refactoring to useuseEffectto avoid suppressing the linting rule, which improves code clarity.Alternative approach:
export const useValueHasChanged = (value: unknown) => { const valueRef = useRef(value); + const prevValueRef = useRef(value); - // eslint-disable-next-line react-hooks/refs - const valueHasChanged = valueRef.current !== value; - // eslint-disable-next-line react-hooks/refs - valueRef.current = value; + + const valueHasChanged = prevValueRef.current !== value; + + useEffect(() => { + prevValueRef.current = value; + }); return valueHasChanged; };eslint.config.ts (1)
32-35: Document why these SonarJS rules are disabled.Four new SonarJS rules have been disabled:
prefer-read-only-propsfunction-return-typeno-redundant-optionalno-useless-intersectionThese rules can catch useful code quality issues. If they're producing false positives in your codebase, consider:
- Adding inline suppressions for legitimate cases rather than global disables
- Documenting why each rule is problematic for your codebase
- Creating a custom rule configuration that better fits your patterns
This helps future maintainers understand the reasoning.
src/server/routers/genre.ts (1)
25-27: Align searchTerm handling with book.ts to avoid undefined in Prisma filterCurrently searchTerm is optional without a default but is used in contains. Prefer trimming and defaulting to '' like in book.ts to avoid undefined and keep behavior consistent.
Apply:
- searchTerm: z.string().optional(), + searchTerm: z.string().trim().optional().prefault(''),src/server/routers/book.ts (1)
134-146: Stricter and consistent validation for genreId (use cuid; optional on create)
- create: handler treats genreId as optional (?? undefined) but schema requires nonempty string.
- update: schema allows any nonempty string; should be cuid to match DB id type.
- .extend({ genreId: z.string().nonempty() }) + .extend({ genreId: z.cuid().optional() })- .extend({ genreId: z.string().nonempty() }) + .extend({ genreId: z.cuid() })The create handler line can stay as-is; the schema change makes it coherent:
genreId: input.genreId ?? undefinedAlso applies to: 174-182, 189-194
package.json (1)
81-82: Nitro nightly in production dependenciesPinned to a nightly build. Nightlies can introduce breaking changes unexpectedly.
- Consider switching to a stable Nitro release before merging to main.
- Verify peer-dep compatibility and runtime by running e2e and a smoke deploy in a staging env.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (54)
.github/workflows/code-quality.yml(1 hunks).storybook/preview.tsx(1 hunks).vscode/settings.example.json(1 hunks)eslint.config.ts(3 hunks)package.json(1 hunks)src/components/form/docs.stories.tsx(1 hunks)src/components/form/field-checkbox-group/docs.stories.tsx(1 hunks)src/components/form/field-checkbox/docs.stories.tsx(1 hunks)src/components/form/field-checkbox/field-checkbox.browser.spec.tsx(1 hunks)src/components/form/field-number/docs.stories.tsx(1 hunks)src/components/form/field-otp/docs.stories.tsx(1 hunks)src/components/form/field-radio-group/docs.stories.tsx(1 hunks)src/components/form/field-text/docs.stories.tsx(1 hunks)src/components/form/form-test-utils.tsx(2 hunks)src/components/ui/calendar.browser.spec.tsx(0 hunks)src/components/ui/calendar.tsx(1 hunks)src/components/ui/date-input.tsx(1 hunks)src/components/ui/popover.stories.tsx(1 hunks)src/components/ui/search-input.tsx(2 hunks)src/components/ui/sidebar.tsx(2 hunks)src/components/ui/theme-switcher.tsx(1 hunks)src/env/client.ts(1 hunks)src/env/server.ts(2 hunks)src/features/account/schema.ts(1 hunks)src/features/auth/schema.ts(2 hunks)src/features/book/schema.ts(2 hunks)src/features/genre/schema.ts(1 hunks)src/features/user/schema.ts(1 hunks)src/hooks/use-value-has-changed.ts(1 hunks)src/lib/i18n/index.ts(0 hunks)src/lib/zod/zod-utils.ts(1 hunks)src/lib/zod/zod-utils.unit.spec.ts(0 hunks)src/locales/ar/auth.json(1 hunks)src/locales/ar/common.json(1 hunks)src/locales/ar/index.ts(0 hunks)src/locales/en/auth.json(1 hunks)src/locales/en/common.json(1 hunks)src/locales/en/index.ts(0 hunks)src/locales/fr/auth.json(1 hunks)src/locales/fr/book.json(0 hunks)src/locales/fr/common.json(1 hunks)src/locales/fr/index.ts(0 hunks)src/locales/sw/auth.json(1 hunks)src/locales/sw/book.json(0 hunks)src/locales/sw/common.json(1 hunks)src/locales/sw/index.ts(0 hunks)src/routes/app/account.index.tsx(1 hunks)src/routes/login/verify.index.tsx(1 hunks)src/routes/manager/account.index.tsx(1 hunks)src/routes/manager/books/index.tsx(1 hunks)src/routes/manager/users/index.tsx(1 hunks)src/server/routers/book.ts(2 hunks)src/server/routers/genre.ts(1 hunks)src/server/routers/user.ts(2 hunks)
💤 Files with no reviewable changes (9)
- src/locales/sw/index.ts
- src/components/ui/calendar.browser.spec.tsx
- src/lib/zod/zod-utils.unit.spec.ts
- src/locales/sw/book.json
- src/locales/ar/index.ts
- src/lib/i18n/index.ts
- src/locales/fr/book.json
- src/locales/fr/index.ts
- src/locales/en/index.ts
🧰 Additional context used
🧬 Code graph analysis (13)
src/components/ui/popover.stories.tsx (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
src/components/form/field-radio-group/docs.stories.tsx (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
src/components/form/docs.stories.tsx (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
src/server/routers/genre.ts (1)
src/features/genre/schema.ts (1)
zGenre(8-15)
src/features/auth/schema.ts (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
src/features/user/schema.ts (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
src/components/form/field-text/docs.stories.tsx (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
src/features/book/schema.ts (2)
src/lib/zod/zod-utils.ts (1)
zu(12-50)src/features/genre/schema.ts (1)
zGenre(8-15)
src/components/ui/search-input.tsx (1)
src/hooks/use-value-has-changed.ts (1)
useValueHasChanged(3-11)
src/components/form/field-otp/docs.stories.tsx (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
src/server/routers/book.ts (1)
src/features/book/schema.ts (1)
zBook(10-19)
src/features/genre/schema.ts (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
src/features/account/schema.ts (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 🔬 Tests (lts/*)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: 🔬 Tests (24)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (42)
src/components/ui/search-input.tsx (2)
51-57: LGTM! Clean debouncing implementation.The debouncing logic correctly uses
useEffectEventto avoid including theonChangecallback in the effect dependencies, which prevents unnecessary effect re-runs when the callback identity changes. The effect will only re-run whensearchordelaychanges, which is the desired behavior.
5-5:useEffectEventsupport confirmed – React 19.2.0 includes this hook as a stable API; no further action required..github/workflows/code-quality.yml (1)
31-31: LGTM! Stricter CI linting enforced.Adding
--max-warnings 0is a good practice that prevents warnings from accumulating and ensures consistent code quality across the codebase..vscode/settings.example.json (1)
9-9: LGTM! Simplified locales configuration.Consolidating to a single locales path aligns with the removal of zod-i18n-map dependency and simplifies the i18n setup.
src/components/ui/sidebar.tsx (2)
76-77: LGTM! Appropriate lint suppression for private state.The eslint-disable is justified here since
_openand_setOpenare intentionally prefixed with underscore to indicate internal/private state that can be overridden by props.
636-637: LGTM! Intentional impurity for random skeleton width.The
react-hooks/puritysuppression is appropriate here since the intentional use ofMath.random()creates visual variety in skeleton loading states, and the randomness doesn't affect functional correctness.src/locales/sw/common.json (1)
26-28: LGTM! Swahili translation added.The addition of the "errors.required" key with the Swahili translation "Inahitajika" is consistent with similar updates across other locale files in this PR.
src/routes/manager/books/index.tsx (1)
11-11: prefault() is valid in Zod v4
prefault() is part of the official Zod v4 API (documented and covered by tests), so this change is correct and requires no further action.src/locales/en/common.json (1)
25-28: LGTM!The addition of the
errors.requiredlocalization key is consistent with similar additions across other locale files (ar, fr, sw) and follows proper JSON structure.src/components/ui/date-input.tsx (1)
36-36: LGTM!The extended ESLint suppression is a mechanical update aligning with broader lint configuration changes in this PR. The underlying code pattern (updating state in
useEffectwhen dependencies change) is valid for this synchronization use case.src/components/ui/theme-switcher.tsx (1)
31-31: LGTM!The extended ESLint suppression is consistent with similar updates across the codebase. The underlying pattern (setting hydration state on mount) is appropriate for detecting client-side rendering.
src/locales/fr/common.json (1)
25-28: LGTM!The French translation for the required error message is correct and maintains consistency with the structure introduced in other locale files.
src/env/client.ts (3)
31-31: Verifyprefault()usage in environment configuration.Similar to the route changes, this uses
prefault()instead ofdefault(). Ensure this is the correct Zod v4 API for default value handling in environment validation.
27-27: No action required:z.url()is correct for Zod v4.
Zod v4 introduces top-level format validators likez.url(), supersedingz.string().url().
37-40: Confirmz.emoji()is correct in Zod v4
In Zod v4,z.emoji()is the standalone validator (the oldz.string().emoji()still works but is deprecated).src/locales/ar/common.json (1)
25-28: LGTM!The Arabic translation for the required error message is properly added and maintains consistency with other locale files.
src/locales/en/auth.json (1)
149-150: LGTM!The new error code for duplicate user email is clearly worded and follows the existing pattern. The addition is consistent with similar updates across other locale files.
src/locales/ar/auth.json (1)
106-107: LGTM!The new error code translation is properly formatted and consistent with the existing error messages in the locale file.
src/env/server.ts (2)
11-11: LGTM!Using
z.url()is the idiomatic way to validate URLs in Zod, replacing the more verbosez.string().url().
45-45: Ignore refine property suggestion Zod v4’srefinemethod useserror(replacingmessagefrom v3), so the current code is correct.Likely an incorrect or invalid review comment.
src/locales/fr/auth.json (1)
119-120: LGTM!The French translation is properly formatted and consistent with other error messages.
src/locales/sw/auth.json (1)
106-107: LGTM!The Swahili translation is properly formatted and consistent with the error code additions in other locales.
src/components/form/form-test-utils.tsx (1)
14-14: LGTM!The updated generic constraint to
ZodType<FieldValues>properly aligns with the Zod migration and provides more specific type safety for form schemas.src/components/ui/calendar.tsx (1)
37-37: LGTM!The lint suppression is appropriately scoped to the specific line where the Icon component is dynamically selected and rendered. This is a reasonable exception to the static-components rule given the pattern matching logic above.
src/components/ui/popover.stories.tsx (1)
59-62: LGTM! Migration to zu.fieldText.required.The migration from
zu.string.nonEmptytozu.fieldText.requiredis consistent with the broader refactor across the codebase. ThefieldTexthelper properly handles empty string transformations and uses standardized error messaging.src/components/form/field-radio-group/docs.stories.tsx (1)
20-25: LGTM! Consistent migration to zu.fieldText.required.The validation has been properly updated to use the
zu.fieldText.requiredhelper with the standardizederrorproperty, maintaining consistency with the broader migration pattern.src/components/form/field-text/docs.stories.tsx (1)
18-21: LGTM! Validation migration is consistent.The field validation has been updated to use
zu.fieldText.required, aligning with the standardized validation approach across all form fields.src/server/routers/user.ts (2)
267-273: Same coerce and prefault pattern - verify consistency.This endpoint uses the same
coerce.number().prefault(20)pattern as the previous endpoint. If the verification for the earlier usage confirms correctness, this usage should be fine as well.
24-31: No action required for coerce + prefault defaults The prefault fallback is applied before coercion, and all current usages (user.ts, genre.ts, book.ts, env/server.ts) behave as expected.src/components/form/docs.stories.tsx (1)
24-28: LGTM! Form validation migrated to fieldText helpers.The form schema has been properly updated to use
zu.fieldText.required()andzu.fieldText.nullish(), leveraging default error messages from the internationalization system. This maintains consistency with the broader validation refactor.src/components/form/field-checkbox/field-checkbox.browser.spec.tsx (1)
16-18: error is correct for Zod v4 refine
Zod v4 uses theerroroption for custom refine messages; no change required.src/routes/app/account.index.tsx (1)
10-14: No action needed: empty string defaults behave identically
Replacing.default('')with.prefault('')has no impact here—""is already valid for bothz.string()andz.enum(['', …]), so the default value passes (or is returned) unchanged. Proceed as is.src/components/form/field-checkbox-group/docs.stories.tsx (1)
17-17: Direct string argument for.nonempty()is valid. Zod 4 accepts.nonempty('message'), so no change is needed.Likely an incorrect or invalid review comment.
src/components/form/field-otp/docs.stories.tsx (1)
19-26: LGTM! Clean migration to zu.fieldText API.The validation correctly migrates to the new
zu.fieldText.required()pattern and pipes through additional length constraints. The approach handles empty strings appropriately via the fieldText transform.src/features/genre/schema.ts (1)
10-11: LGTM! Consistent migration to Zod 4 APIs.Both changes align with the migration strategy:
- Using native
z.cuid()matches Zod 4's improved type-specific validators- Adopting
zu.fieldText.required()follows the project-wide pattern for text field validationsrc/features/account/schema.ts (1)
10-10: LGTM! Consistent fieldText migration.The update correctly adopts the
zu.fieldText.required()pattern, maintaining the same validation behavior while aligning with the project-wide API changes.src/features/auth/schema.ts (2)
10-13: LGTM! Consolidated error handling.The simplified error function correctly distinguishes between missing input (
!issue.input) and invalid input, maintaining the same validation messages with cleaner code.
20-27: LGTM! Correct email validation migration.The migration to
zu.fieldText.required().pipe(z.email(...))properly handles empty strings via the fieldText transform and provides contextual error messages through the error function.src/features/user/schema.ts (1)
12-19: LGTM! Proper fieldText migration for nullable and required fields.Both updates correctly apply the migration pattern:
nameuseszu.fieldText.nullish()for the optional field, transforming empty strings tonullzu.fieldText.required().pipe(z.email(...))with contextual error messages that distinguish missing vs. invalid inputsrc/features/book/schema.ts (3)
12-12: LGTM! Updated to Zod 4 native CUID validator.The change to
z.cuid()adopts Zod 4's built-in type-specific validator, which is more idiomatic than the previousz.string().cuid()pattern.
13-16: LGTM! Consistent fieldText migration across fields.All text fields correctly migrate to the appropriate
zu.fieldTextvariants:
titleandauthor:zu.fieldText.required()for mandatory fieldspublisher:zu.fieldText.nullish()for optional fieldsEach variant properly handles empty strings according to field optionality.
25-25: LGTM! Consistent validation for form field.The
genreIdfield correctly adoptszu.fieldText.required(), maintaining consistency with the other required text fields in the schema.
| import react from '@eslint-react/eslint-plugin'; | ||
| import tanstackQuery from '@tanstack/eslint-plugin-query'; | ||
| import tanstackRouter from '@tanstack/eslint-plugin-router'; | ||
| import { defineConfig } from 'eslint/config'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Address the TypeScript errors properly rather than suppressing them.
The @ts-expect-error comments indicate type mismatches that are being suppressed rather than resolved. While these may be due to incomplete type definitions in the ESLint packages, consider:
- Verifying you're using compatible versions of the ESLint plugins
- Checking if newer versions have resolved these type issues
- Adding proper type declarations if the packages lack them
Suppressing type errors can hide breaking changes or incorrect usage patterns.
Also applies to: 22-23
🏁 Script executed:
#!/bin/bash
# Check the versions of ESLint packages
echo "=== Checking ESLint package versions ==="
cat package.json | jq '.devDependencies | to_entries | map(select(.key | contains("eslint"))) | from_entries'Length of output: 459
Use defineFlatConfig and flat.configs.recommended from @eslint/js instead of suppressing TS errors
Replace
import { defineConfig } from 'eslint/config';with
import { defineFlatConfig, flat } from '@eslint/js';then update your config to use defineFlatConfig({ extends: [flat.configs.recommended], … }) and remove all @ts-expect-error comments.
🤖 Prompt for AI Agents
In eslint.config.ts around line 5, replace the current import from
'eslint/config' and the use of defineConfig with imports from '@eslint/js' and
defineFlatConfig; specifically import defineFlatConfig and flat from
'@eslint/js', change the exported config to use defineFlatConfig({ extends:
[flat.configs.recommended], ... }) and remove any @ts-expect-error comments
related to the old API; ensure the new shape matches flat config expectations
(top-level fields like ignores, plugins, rules, and overrides) so TypeScript
errors are resolved.
| lovesBears: z.boolean().refine((val) => val === true, { | ||
| message: 'Please say you love bears.', | ||
| error: 'Please say you love bears.', | ||
| }), |
There was a problem hiding this comment.
Use refine’s message option (not error)
Zod’s refine expects { message }; { error } will be ignored and no message will surface in the form.
- lovesBears: z.boolean().refine((val) => val === true, {
- error: 'Please say you love bears.',
- }),
+ lovesBears: z.boolean().refine((val) => val === true, {
+ message: 'Please say you love bears.',
+ }),📝 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.
| lovesBears: z.boolean().refine((val) => val === true, { | |
| message: 'Please say you love bears.', | |
| error: 'Please say you love bears.', | |
| }), | |
| lovesBears: z.boolean().refine((val) => val === true, { | |
| message: 'Please say you love bears.', | |
| }), |
🤖 Prompt for AI Agents
In src/components/form/field-checkbox/docs.stories.tsx around lines 26 to 28,
the Zod refine call uses the invalid option { error: 'Please say you love
bears.' } which is ignored; replace it with the correct option { message:
'Please say you love bears.' } so the validation message surfaces in the form.
src/env/server.ts
Outdated
| SESSION_EXPIRATION_IN_SECONDS: z.coerce.number().int().prefault(2592000), // 30 days by default | ||
| SESSION_UPDATE_AGE_IN_SECONDS: z.coerce.number().int().prefault(86400), // 1 day by default |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that prefault() is a valid Zod method.
The migration from default() to prefault() is used throughout the codebase, but prefault() does not appear in standard Zod documentation or the retrieved learnings. This could be:
- A typo (should be
default()?) - A custom extension defined in the codebase
- An undocumented Zod 4 feature
Please verify this is correct, as using an undefined method would cause runtime errors.
Run the following script to check if prefault is defined in the codebase:
Also applies to: 25-25, 28-28
🏁 Script executed:
#!/bin/bash
# Search for prefault definition or import in the codebase
echo "=== Searching for prefault definition or declaration ==="
rg -n "prefault\s*[:=]" --type ts --type tsx
echo -e "\n=== Searching for prefault in dependencies ==="
rg -n "prefault" node_modules/zod/lib -A 2 2>/dev/null || echo "Not found in zod package"
echo -e "\n=== Checking if it's a custom extension ==="
ast-grep --pattern 'prefault($$$) { $$$ }'Length of output: 202
Replace prefault() with a valid default or define it via a Zod extension
.prefault() isn’t defined in the codebase or Zod package and will break at runtime. Either rename these calls to .default() or add the corresponding prefault() extension before use.
🤖 Prompt for AI Agents
In src/env/server.ts around lines 13 to 14, the schema uses a non-existent
.prefault() which will throw at runtime; replace those calls with
.default(2592000) and .default(86400) respectively so the lines read
z.coerce.number().int().default(2592000) and
z.coerce.number().int().default(86400); alternatively, if you intentionally need
a custom prefault behavior, add and register a Zod extension that implements
prefault() before using it.
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the prefault() method in Zod 4? How does it differ from default() in Zod?
💡 Result:
Short answer: prefault is a "pre-parse default" that is applied by inserting a fallback into the input before the inner schema runs; default short‑circuits and returns the default without running the inner schema.
Details and examples
- .default(value): if the input === undefined, Zod 4 returns value immediately (parsing is short‑circuited). The default must be assignable to the schema's output type. [1][2]
- .prefault(value): if the input === undefined, Zod will replace the input with value and then run the inner schema as usual (so transforms/checks on the schema will run against the prefaulted value). The prefault value must be assignable to the schema's input type. This reproduces Zod 3's old behavior. Example pattern shown in Zod discussions:
const params = z.object({ limit: z.number().default(0) });
const prefaulted = params.prefault({});
prefaulted.parse(undefined); // => { limit: 0 } (inner parsing runs). [2][3]
When to use which
- Use .default when you want a cheap, final fallback and you don't want the inner schema to run on the fallback.
- Use .prefault when you want the default to be parsed/validated/transformed by the inner schema (i.e., "fill then parse").
Sources
[1] Zod v4 changelog / migration notes (explains default short‑circuit change and prefault concept).
[2] GitHub discussion/examples showing prefault pattern and why default changed.
[3] "Introducing Zod Codecs" (notes default/prefault behavior in v4).
@ivan-dalmet, thank you for the clarification! You're absolutely right. I've confirmed that prefault() is indeed a valid Zod v4 method.
For context, prefault() differs from default() in Zod v4:
.default(value): Short-circuits parsing and returns the default immediately when input is undefined (the default value is never validated/transformed).prefault(value): Replaces undefined input with the fallback value, then runs the full schema validation/transformation on it (reproduces Zod 3's olddefault()behavior)
The usage in this file is correct – prefault() ensures the default values (2592000 and 86400) still go through the .coerce.number().int() validation chain. Apologies for the false positive! 🎉
✏️ Learnings added
Learnt from: ivan-dalmet
PR: BearStudio/start-ui-web#620
File: src/env/server.ts:13-14
Timestamp: 2025-10-09T08:30:25.898Z
Learning: In Zod v4, `prefault()` is a valid method that acts as a "pre-parse default". It differs from `default()` by applying the fallback value before the inner schema runs, whereas `default()` short-circuits parsing. Usage like `z.coerce.number().int().prefault(2592000)` is correct and ensures the default value goes through the validation chain.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| export const zu = { | ||
| string: { | ||
| nonEmpty( | ||
| s: ZodString, | ||
| options: { | ||
| required_error?: string; | ||
| } = {} | ||
| ) { | ||
| return s | ||
| .trim() | ||
| .min( | ||
| 1, | ||
| options.required_error ?? | ||
| t('zod:errors.invalid_type_received_undefined') | ||
| ); | ||
| }, | ||
| nonEmptyNullable( | ||
| s: ZodString, | ||
| options: { | ||
| required_error?: string; | ||
| } = {} | ||
| ) { | ||
| return z | ||
| .literal('') | ||
| .transform(() => null) | ||
| .or(zu.string.nonEmpty(s, options).nullable()); | ||
| }, | ||
| nonEmptyNullish( | ||
| s: ZodString, | ||
| options: { | ||
| required_error?: string; | ||
| } = {} | ||
| ) { | ||
| return z | ||
| .literal('') | ||
| .transform(() => null) | ||
| .or(zu.string.nonEmpty(s, options).nullish()); | ||
| }, | ||
| email( | ||
| s: ZodString, | ||
| options: { | ||
| required_error?: string; | ||
| invalid_type_error?: string; | ||
| } = {} | ||
| ) { | ||
| return zu.string | ||
| .nonEmpty(s.toLowerCase(), options) | ||
| .email(options.invalid_type_error); | ||
| }, | ||
| emailNullable( | ||
| s: ZodString, | ||
| options: { | ||
| required_error?: string; | ||
| invalid_type_error?: string; | ||
| } = {} | ||
| ) { | ||
| return zu.string.nonEmptyNullable(zu.string.email(s, options), options); | ||
| }, | ||
| emailNullish( | ||
| s: ZodString, | ||
| options: { | ||
| required_error?: string; | ||
| invalid_type_error?: string; | ||
| } = {} | ||
| ) { | ||
| return zu.string.nonEmptyNullish(zu.string.email(s, options), options); | ||
| }, | ||
| }, | ||
| array: { | ||
| nonEmpty<T extends Schema>(a: ZodArray<T>, message?: string) { | ||
| return a.min( | ||
| 1, | ||
| message ?? t('zod:errors.invalid_type_received_undefined') | ||
| ); | ||
| }, | ||
| nonEmptyNullable<T extends Schema>(a: ZodArray<T>, message?: string) { | ||
| return a | ||
| .length(0) | ||
| .transform(() => null) | ||
| .or(zu.array.nonEmpty(a, message).nullable()); | ||
| }, | ||
| nonEmptyNullish<T extends Schema>(a: ZodArray<T>, message?: string) { | ||
| return a | ||
| .length(0) | ||
| .transform(() => null) | ||
| .or(zu.array.nonEmpty(a, message).nullish()); | ||
| }, | ||
| fieldText: { | ||
| required: ( | ||
| params: Parameters<typeof z.string>[0] = { | ||
| error: t('common:errors.required'), | ||
| } | ||
| ) => z.string(params).transform(emptyStringAsNull).pipe(z.string(params)), | ||
| nullable: ( | ||
| params: Parameters<typeof z.string>[0] = { | ||
| error: t('common:errors.required'), | ||
| } | ||
| ) => | ||
| z | ||
| .string(params) | ||
| .transform(emptyStringAsNull) | ||
| .nullable() | ||
| .pipe(z.string(params).nullable()), | ||
| nullish: ( | ||
| params: Parameters<typeof z.string>[0] = { | ||
| error: t('common:errors.required'), | ||
| } | ||
| ) => | ||
| z | ||
| .string(params) | ||
| .transform(emptyStringAsNull) | ||
| .nullish() | ||
| .pipe(z.string(params).nullish()), | ||
| optional: ( | ||
| params: Parameters<typeof z.string>[0] = { | ||
| error: t('common:errors.required'), | ||
| } | ||
| ) => | ||
| z | ||
| .string(params) | ||
| .transform(emptyStringAsUndefined) | ||
| .optional() | ||
| .pipe(z.string(params).optional()), | ||
| }, |
There was a problem hiding this comment.
Map custom { error } to Zod’s expected params; fix type and message wiring
z.string doesn’t accept an error property; it expects required_error/invalid_type_error (or errorMap). Current default { error: ... } will not work.
Apply:
-import z from 'zod';
+import z from 'zod';
-const emptyStringAsNull = (input: string) =>
- // Cast null value to string for React Hook Form inference
+const emptyStringAsNull = (input: string) =>
+ // Cast null value to string for React Hook Form inference
input.trim() === '' ? (null as unknown as string) : input.trim();
-const emptyStringAsUndefined = (input: string) =>
- // Cast null value to string for React Hook Form inference
+const emptyStringAsUndefined = (input: string) =>
+ // Cast undefined value to string for React Hook Form inference
input.trim() === '' ? (undefined as unknown as string) : input.trim();
export const zu = {
- fieldText: {
- required: (
- params: Parameters<typeof z.string>[0] = {
- error: t('common:errors.required'),
- }
- ) => z.string(params).transform(emptyStringAsNull).pipe(z.string(params)),
- nullable: (
- params: Parameters<typeof z.string>[0] = {
- error: t('common:errors.required'),
- }
- ) =>
- z
- .string(params)
- .transform(emptyStringAsNull)
- .nullable()
- .pipe(z.string(params).nullable()),
- nullish: (
- params: Parameters<typeof z.string>[0] = {
- error: t('common:errors.required'),
- }
- ) =>
- z
- .string(params)
- .transform(emptyStringAsNull)
- .nullish()
- .pipe(z.string(params).nullish()),
- optional: (
- params: Parameters<typeof z.string>[0] = {
- error: t('common:errors.required'),
- }
- ) =>
- z
- .string(params)
- .transform(emptyStringAsUndefined)
- .optional()
- .pipe(z.string(params).optional()),
- },
+ fieldText: {
+ required: (opts?: { error?: string }) => {
+ const p = opts?.error
+ ? { required_error: opts.error, invalid_type_error: opts.error }
+ : undefined;
+ return z.string(p).transform(emptyStringAsNull).pipe(z.string(p));
+ },
+ nullable: (opts?: { error?: string }) => {
+ const p = opts?.error
+ ? { required_error: opts.error, invalid_type_error: opts.error }
+ : undefined;
+ return z.string(p).transform(emptyStringAsNull).nullable().pipe(z.string(p).nullable());
+ },
+ nullish: (opts?: { error?: string }) => {
+ const p = opts?.error
+ ? { required_error: opts.error, invalid_type_error: opts.error }
+ : undefined;
+ return z.string(p).transform(emptyStringAsNull).nullish().pipe(z.string(p).nullish());
+ },
+ optional: (opts?: { error?: string }) => {
+ const p = opts?.error
+ ? { required_error: opts.error, invalid_type_error: opts.error }
+ : undefined;
+ return z.string(p).transform(emptyStringAsUndefined).optional().pipe(z.string(p).optional());
+ },
+ },
};Additionally, consider standardizing on import { z } from 'zod' across the repo to avoid relying on default export nuances.
📝 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.
| export const zu = { | |
| string: { | |
| nonEmpty( | |
| s: ZodString, | |
| options: { | |
| required_error?: string; | |
| } = {} | |
| ) { | |
| return s | |
| .trim() | |
| .min( | |
| 1, | |
| options.required_error ?? | |
| t('zod:errors.invalid_type_received_undefined') | |
| ); | |
| }, | |
| nonEmptyNullable( | |
| s: ZodString, | |
| options: { | |
| required_error?: string; | |
| } = {} | |
| ) { | |
| return z | |
| .literal('') | |
| .transform(() => null) | |
| .or(zu.string.nonEmpty(s, options).nullable()); | |
| }, | |
| nonEmptyNullish( | |
| s: ZodString, | |
| options: { | |
| required_error?: string; | |
| } = {} | |
| ) { | |
| return z | |
| .literal('') | |
| .transform(() => null) | |
| .or(zu.string.nonEmpty(s, options).nullish()); | |
| }, | |
| email( | |
| s: ZodString, | |
| options: { | |
| required_error?: string; | |
| invalid_type_error?: string; | |
| } = {} | |
| ) { | |
| return zu.string | |
| .nonEmpty(s.toLowerCase(), options) | |
| .email(options.invalid_type_error); | |
| }, | |
| emailNullable( | |
| s: ZodString, | |
| options: { | |
| required_error?: string; | |
| invalid_type_error?: string; | |
| } = {} | |
| ) { | |
| return zu.string.nonEmptyNullable(zu.string.email(s, options), options); | |
| }, | |
| emailNullish( | |
| s: ZodString, | |
| options: { | |
| required_error?: string; | |
| invalid_type_error?: string; | |
| } = {} | |
| ) { | |
| return zu.string.nonEmptyNullish(zu.string.email(s, options), options); | |
| }, | |
| }, | |
| array: { | |
| nonEmpty<T extends Schema>(a: ZodArray<T>, message?: string) { | |
| return a.min( | |
| 1, | |
| message ?? t('zod:errors.invalid_type_received_undefined') | |
| ); | |
| }, | |
| nonEmptyNullable<T extends Schema>(a: ZodArray<T>, message?: string) { | |
| return a | |
| .length(0) | |
| .transform(() => null) | |
| .or(zu.array.nonEmpty(a, message).nullable()); | |
| }, | |
| nonEmptyNullish<T extends Schema>(a: ZodArray<T>, message?: string) { | |
| return a | |
| .length(0) | |
| .transform(() => null) | |
| .or(zu.array.nonEmpty(a, message).nullish()); | |
| }, | |
| fieldText: { | |
| required: ( | |
| params: Parameters<typeof z.string>[0] = { | |
| error: t('common:errors.required'), | |
| } | |
| ) => z.string(params).transform(emptyStringAsNull).pipe(z.string(params)), | |
| nullable: ( | |
| params: Parameters<typeof z.string>[0] = { | |
| error: t('common:errors.required'), | |
| } | |
| ) => | |
| z | |
| .string(params) | |
| .transform(emptyStringAsNull) | |
| .nullable() | |
| .pipe(z.string(params).nullable()), | |
| nullish: ( | |
| params: Parameters<typeof z.string>[0] = { | |
| error: t('common:errors.required'), | |
| } | |
| ) => | |
| z | |
| .string(params) | |
| .transform(emptyStringAsNull) | |
| .nullish() | |
| .pipe(z.string(params).nullish()), | |
| optional: ( | |
| params: Parameters<typeof z.string>[0] = { | |
| error: t('common:errors.required'), | |
| } | |
| ) => | |
| z | |
| .string(params) | |
| .transform(emptyStringAsUndefined) | |
| .optional() | |
| .pipe(z.string(params).optional()), | |
| }, | |
| import z from 'zod'; | |
| const emptyStringAsNull = (input: string) => | |
| // Cast null value to string for React Hook Form inference | |
| input.trim() === '' ? (null as unknown as string) : input.trim(); | |
| const emptyStringAsUndefined = (input: string) => | |
| // Cast undefined value to string for React Hook Form inference | |
| input.trim() === '' ? (undefined as unknown as string) : input.trim(); | |
| export const zu = { | |
| fieldText: { | |
| required: (opts?: { error?: string }) => { | |
| const p = opts?.error | |
| ? { required_error: opts.error, invalid_type_error: opts.error } | |
| : undefined; | |
| return z.string(p).transform(emptyStringAsNull).pipe(z.string(p)); | |
| }, | |
| nullable: (opts?: { error?: string }) => { | |
| const p = opts?.error | |
| ? { required_error: opts.error, invalid_type_error: opts.error } | |
| : undefined; | |
| return z | |
| .string(p) | |
| .transform(emptyStringAsNull) | |
| .nullable() | |
| .pipe(z.string(p).nullable()); | |
| }, | |
| nullish: (opts?: { error?: string }) => { | |
| const p = opts?.error | |
| ? { required_error: opts.error, invalid_type_error: opts.error } | |
| : undefined; | |
| return z | |
| .string(p) | |
| .transform(emptyStringAsNull) | |
| .nullish() | |
| .pipe(z.string(p).nullish()); | |
| }, | |
| optional: (opts?: { error?: string }) => { | |
| const p = opts?.error | |
| ? { required_error: opts.error, invalid_type_error: opts.error } | |
| : undefined; | |
| return z | |
| .string(p) | |
| .transform(emptyStringAsUndefined) | |
| .optional() | |
| .pipe(z.string(p).optional()); | |
| }, | |
| }, | |
| }; |
| state: z.enum(['', 'change-name']).prefault(''), | ||
| newEmail: z.string().prefault(''), |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the Zod v4 API and prefault() method.
The change from .default('') to .prefault('') suggests a Zod v4 API change. Please verify that:
- Zod v4 has been released and is the version being used
prefault()is the correct Zod v4 method for default values- The behavior is equivalent to v3's
default()
🌐 Web query:
What is the latest version of Zod and does Zod v4 introduce a prefault() method to replace default()?
💡 Result:
As of Oct 9, 2025 the Zod 4 release line is the current stable major (latest published release in the 4.x series is v4.1.12). [1]
No — Zod v4 does not add a prefault() API. Zod 4 intentionally changed how .default() behaves (it short-circuits and returns the default rather than "prefaulting" and reparsing the default); the maintainer notes that the old "prefault" behavior was removed and can be reproduced with z.transform if needed. [2][3]
Sources:
- Latest Zod v4 releases (release listings). [1]
- GitHub discussion explaining removal of the "prefault" behavior and how to emulate it with z.transform. [2]
- Zod v4 docs / announcement. [3]
Replace .prefault() with .default() for Zod v4 compatibility
prefault() was removed in v4; use .default('') (short-circuits and returns the default) or, to emulate the old “prefault” behavior, wrap in z.transform.
🤖 Prompt for AI Agents
In src/routes/manager/account.index.tsx around lines 11 to 12, the schema uses
the removed Zod v4 method `.prefault('')`; replace `.prefault('')` with
`.default('')` on both fields (e.g., state: z.enum(['',
'change-name']).default('') and newEmail: z.string().default('')), and if you
need the old “prefault” transformation behavior instead of a simple default,
wrap the schema in z.transform to apply the transformation after parsing.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/features/auth/schema.ts (1)
40-40: Use zu.fieldText.required() for consistency and Zod v4 compatibility.The
namefield usesz.string().nonempty(), which:
- Is inconsistent with the
zu.fieldText.required()- Uses
.nonempty()which is deprecated in Zod v4 in favor of.min(1)or built-in error messages- Lacks the empty-string-to-null transformation that
fieldTextprovides for React Hook Form compatibilityApply this diff for consistency:
- name: z.string().nonempty(), + name: zu.fieldText.required(),
🧹 Nitpick comments (3)
src/lib/zod/zod-utils.ts (2)
2-2: Standardize on named import for consistency.The codebase uses named imports (
import { z } from 'zod') elsewhere (e.g.,src/features/user/schema.ts,src/features/auth/schema.ts), but this file uses a default import.Apply this diff to align with the rest of the codebase:
-import z from 'zod'; +import { z } from 'zod';
8-10: Fix the comment to accurately describe undefined casting.The comment on line 9 states "Cast null value to string" but the function returns
undefined, notnull.Apply this diff to fix the comment:
const emptyStringAsUndefined = (input: string) => - // Cast null value to string for React Hook Form inference + // Cast undefined value to string for React Hook Form inference input.trim() === '' ? (undefined as unknown as string) : input.trim();src/features/auth/schema.ts (1)
9-12: Consider consistent error parameter style.This code uses the object syntax
z.string({ error: ... })while thezu.fieldTexthelpers insrc/lib/zod/zod-utils.ts(lines 15, 18, 26, 34) pass error strings directly. Both are valid in Zod v4, but consistency across the codebase would improve maintainability.If you prefer consistency, apply this diff:
- z - .string({ - error: t('auth:common.otp.required'), - }) + z + .string(t('auth:common.otp.required')) .length(6, t('auth:common.otp.invalidLength'));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/features/auth/schema.ts(2 hunks)src/features/user/schema.ts(1 hunks)src/lib/zod/zod-utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/features/user/schema.ts (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
src/features/auth/schema.ts (1)
src/lib/zod/zod-utils.ts (1)
zu(12-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 🔬 Tests (lts/*)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: 🔬 Tests (24)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (3)
src/lib/zod/zod-utils.ts (1)
12-42: LGTM: Past review concerns addressed!The past review flagged that
z.stringdoesn't accept an{ error: ... }object in earlier Zod versions. The current implementation correctly passes strings directly as parameters (lines 15, 18, 26, 34), which is valid Zod v4 syntax wherez.string('error message')sets the error message for all string validations.The
fieldTexthelpers properly:
- Transform empty strings to null/undefined for React Hook Form inference
- Chain with
.pipe()to enforce nullability constraints (required/nullable/nullish/optional)- Apply consistent error messaging across the codebase
src/features/user/schema.ts (1)
12-20: LGTM: Clean migration to fieldText pattern.The schema correctly adopts the new
zu.fieldTexthelpers:
nameusesnullish()for optional handling with empty-string normalizationrequired()chained withz.email()and provides contextual error messages by checkingissue.inputto distinguish between missing input (required error) and invalid format (invalid error)This pattern is consistent with the broader PR migration strategy.
src/features/auth/schema.ts (1)
17-24: LGTM: Email field correctly migrated to fieldText pattern.The email validation properly uses
zu.fieldText.required()chained withz.email(), and the error handler appropriately distinguishes between missing input and invalid format by checkingissue.input.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lib/zod/zod-utils.ts (1)
14-40: Wrap default parameter in{ error: ... }object for Zod v4.The default parameter value is a plain string, but Zod v4's
z.string()expects an object with anerrorproperty. The current code will fail at runtime because it passes the string directly instead of wrapping it.Based on learnings: In Zod v4, use the unified
errorparameter (e.g.,z.string({ error: 'Required' })) instead of the older Zod v3 pattern (required_error,invalid_type_error).Apply this diff to fix all four methods:
fieldText: { required: ( - params: Parameters<typeof z.string>[0] = t('common:errors.required') + params: Parameters<typeof z.string>[0] = { error: t('common:errors.required') } ) => z.string(params).transform(emptyStringAsNull).pipe(z.string(params)), nullable: ( - params: Parameters<typeof z.string>[0] = t('common:errors.required') + params: Parameters<typeof z.string>[0] = { error: t('common:errors.required') } ) => z .string(params) .transform(emptyStringAsNull) .nullable() .pipe(z.string(params).nullable()), nullish: ( - params: Parameters<typeof z.string>[0] = t('common:errors.required') + params: Parameters<typeof z.string>[0] = { error: t('common:errors.required') } ) => z .string(params) .transform(emptyStringAsNull) .nullish() .pipe(z.string(params).nullish()), optional: ( - params: Parameters<typeof z.string>[0] = t('common:errors.required') + params: Parameters<typeof z.string>[0] = { error: t('common:errors.required') } ) => z .string(params) .transform(emptyStringAsUndefined) .optional() .pipe(z.string(params).optional()), },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/features/auth/schema.ts(2 hunks)src/lib/zod/zod-utils.ts(1 hunks)src/server/routers/book.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/auth/schema.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-09T08:29:28.532Z
Learnt from: ivan-dalmet
PR: BearStudio/start-ui-web#620
File: src/components/form/field-number/docs.stories.tsx:23-23
Timestamp: 2025-10-09T08:29:28.532Z
Learning: In Zod v4, the unified `error` parameter is used for custom error messages. For example, `z.number({ error: 'Required' })` is the correct syntax, replacing the older Zod v3 pattern of `required_error` and `invalid_type_error`.
Applied to files:
src/lib/zod/zod-utils.ts
🧬 Code graph analysis (1)
src/server/routers/book.ts (1)
src/features/book/schema.ts (2)
zBook(10-19)zFormFieldsBook(22-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (5)
src/server/routers/book.ts (5)
4-4: LGTM! Good addition of shared schema import.Adding
zFormFieldsBookenables better schema reuse across routes, reducing duplication.
30-36: LGTM! Consistent use ofz.cuid()shorthand.The output schema correctly uses the simplified
z.cuid()syntax, matching the input cursor field.
96-100: LGTM! Correct migration toz.cuid()shorthand.The input schema correctly uses the simplified
z.cuid()syntax for the id field.
127-127: LGTM! Good use of shared schema.Replacing the inline
.pick()withzFormFieldsBook()promotes code reuse and maintainability.
21-29: Approve prefault() usage: prefault provides Zod 3–compatible pre-parse defaults, so this migration is correct.
| }) | ||
| .extend({ genreId: z.string().nonempty() }) | ||
| ) | ||
| .input(zFormFieldsBook().extend({ id: z.string() })) |
There was a problem hiding this comment.
Fix inconsistent id type validation.
The id field uses z.string() here, but other routes use z.cuid():
- Line 98 (
getById):z.cuid() - Lines 205-207 (
deleteById):z.cuid()(viazBook().pick({ id: true })) - Schema definition:
zBook()hasid: z.cuid()
This inconsistency allows updateById to accept any string, not just valid CUIDs, potentially passing invalid IDs to the database handler.
Apply this diff to fix the inconsistency:
- .input(zFormFieldsBook().extend({ id: z.string() }))
+ .input(zFormFieldsBook().extend({ id: z.cuid() }))📝 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.
| .input(zFormFieldsBook().extend({ id: z.string() })) | |
| .input(zFormFieldsBook().extend({ id: z.cuid() })) |
🤖 Prompt for AI Agents
In src/server/routers/book.ts around line 165, the updateById input uses
.input(zFormFieldsBook().extend({ id: z.string() })) which allows any string for
id; change it to use z.cuid() to match other routes and the zBook() schema by
replacing z.string() with z.cuid(), or better yet extend using zBook().pick({
id: true }) or reuse the existing id schema so updateById only accepts valid
CUIDs before calling the database handler.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/routers/genre.ts (1)
50-58: Fix pagination: add skip and deterministic tie-breakerWithout skip, the cursor item reappears on the next page; ordering only by name can be unstable. Add skip and a secondary id sort.
- context.db.genre.findMany({ - // Get an extra item at the end which we'll use as next cursor - take: input.limit + 1, - cursor: input.cursor ? { id: input.cursor } : undefined, - orderBy: { - name: 'asc', - }, - where, - }), + context.db.genre.findMany({ + // Get an extra item at the end which we'll use as next cursor + take: input.limit + 1, + cursor: input.cursor ? { id: input.cursor } : undefined, + skip: input.cursor ? 1 : 0, + orderBy: [{ name: 'asc' }, { id: 'asc' }], + where, + }),
♻️ Duplicate comments (1)
src/server/routers/book.ts (1)
165-165: Past review comment is outdated but underlying concern remains valid.The previous review flagged the use of
z.string()instead ofz.cuid()for inconsistency with other routes. However, thezBook()schema has since been updated to useid: z.string()(seesrc/features/book/schema.ts:11), making this change consistent across all routes.The underlying concern about accepting any string for IDs instead of validated CUIDs remains valid. Verify this is intentional (see comment on line 98).
🧹 Nitpick comments (1)
src/server/routers/genre.ts (1)
23-23: Cursor validation relaxed — consider minimal guard or ID-shape checkDropping .cuid allows any string; Prisma will throw if the cursor record is missing/invalid. At least prevent empty strings (and, if applicable, restore an ID-shape check).
- cursor: z.string().optional(), + cursor: z.string().trim().min(1).optional(),If your ids are cuid2/uuid, prefer the corresponding validator instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/form/field-checkbox-group/docs.stories.tsx(1 hunks)src/server/routers/book.ts(6 hunks)src/server/routers/genre.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/form/field-checkbox-group/docs.stories.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/server/routers/genre.ts (1)
src/features/genre/schema.ts (1)
zGenre(8-15)
src/server/routers/book.ts (1)
src/features/book/schema.ts (2)
zBook(10-19)zFormFieldsBook(22-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🔬 Tests (24)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (8)
src/server/routers/book.ts (5)
4-4: LGTM - Import addition aligns with schema refactoring.The addition of
zFormFieldsBookimport is necessary for the updatedcreateandupdateByIdroute inputs and aligns with the schema consolidation.
98-98: Verify ID validation weakening is intentional.The
idfield validation changed fromz.string().cuid()toz.string(), which is now consistent with the updatedzBook()schema (seesrc/features/book/schema.ts:11). However, this allows any string as an ID instead of requiring valid CUIDs.Confirm this is part of the intentional Zod 4 migration strategy and not an oversight. If database IDs are still CUIDs, consider retaining CUID validation to catch invalid inputs early.
127-127: LGTM - Schema refactoring improves reusability.Using
zFormFieldsBook()consolidates the input schema and improves maintainability by reusing the shared form fields schema defined insrc/features/book/schema.ts:21-24.
196-196: LGTM - Permission correction improves security.The change from
user: ['delete']tobook: ['delete']correctly aligns the permission check with the operation. Deleting a book should require book deletion rights, not user deletion rights.
25-26: Replace.prefault()with.default()in Zod schemas
Zod 4 removed.prefault(); use.default()(short-circuits parsing) or, if you need pre-parse defaults, apply a.transform(v => v ?? default)/.pipe() after the schema.
Locations: src/server/routers/book.ts lines 25–26, 28⛔ Skipped due to learnings
Learnt from: ivan-dalmet PR: BearStudio/start-ui-web#620 File: src/env/server.ts:13-14 Timestamp: 2025-10-09T08:30:25.898Z Learning: In Zod v4, `prefault()` is a valid method that acts as a "pre-parse default". It differs from `default()` by applying the fallback value before the inner schema runs, whereas `default()` short-circuits parsing. Usage like `z.coerce.number().int().prefault(2592000)` is correct and ensures the default value goes through the validation chain.src/server/routers/genre.ts (3)
24-24: Defaults migration LGTM.prefault(20) and .prefault({}) read well and align with the zod v4 changes.
Also applies to: 27-27
32-32: Output nextCursor type change is consistentAligns with input cursor and zGenre id: string. Looks good.
37-37: Corrected log messageAccurate and clearer. LGTM.
| cursor: z.string().cuid().optional(), | ||
| limit: z.coerce.number().int().min(1).max(100).default(20), | ||
| searchTerm: z.string().trim().optional().default(''), | ||
| cursor: z.string().optional(), |
There was a problem hiding this comment.
🧩 Analysis chain
Verify weakened cursor validation is intentional.
The removal of .cuid() validation on cursor fields means the API now accepts any string instead of requiring valid CUIDs. Since line 64 uses input.cursor directly in the Prisma where clause { id: input.cursor }, accepting invalid cursor formats could cause runtime errors or database query failures.
Confirm this change is intentional and that the database ID column does not enforce CUID format.
Also applies to: 33-33
🏁 Script executed:
#!/bin/bash
# Check if Book.id field is defined as a CUID type in the Prisma schema
fd -e prisma -x cat {} \; | rg -n -A2 -B2 'model Book'Length of output: 109
Reinstate CUID validation on cursor and correct .prefault() usage
- cursor fields (lines 24, 33): use
z.string().cuid().optional()to match Prisma’s@default(cuid()) - replace all
.prefault()calls with.default()(valid Zod API)
🤖 Prompt for AI Agents
In src/server/routers/book.ts around lines 24 and 33, the cursor schema should
reinstate CUID validation and any misspelled Zod default usage must be fixed:
change cursor fields from z.string().optional() to z.string().cuid().optional()
to match Prisma @default(cuid()), and replace all occurrences of the invalid
.prefault() method with the correct Zod .default() API, ensuring default values
are provided to .default(...) where appropriate.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.json(1 hunks)src/locales/ar/index.ts(1 hunks)src/locales/en/index.ts(1 hunks)src/locales/fr/index.ts(1 hunks)src/locales/sw/index.ts(1 hunks)vite.config.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- vite.config.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: 🔬 Tests (lts/*)
- GitHub Check: 🔬 Tests (24)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (8)
src/locales/ar/index.ts (2)
3-13: LGTM: JSON import syntax modernized.Switching from
assert { type: 'json' }towith { type: 'json' }aligns with the Stage 4 ECMAScript standard for import attributes. The deprecatedassertsyntax is correctly replaced.
15-27: Safe to removezodexport – searches show no imports or references tolocale.zodin the codebase.src/locales/en/index.ts (2)
3-13: LGTM: Import attributes modernized.The JSON imports correctly adopt the
with { type: 'json' }syntax, replacing the deprecatedassertsyntax. This is the standard import attribute syntax.
15-27: No.zodreferences remain. Removal of thezodexport fromsrc/locales/en/index.tsdoes not break existing code.src/locales/fr/index.ts (2)
3-13: LGTM: JSON imports updated to standard syntax.The migration from
asserttowith { type: 'json' }correctly adopts the Stage 4 import attributes specification.
15-27:zodremoval fromfrlocale is safe. No code imports or accessesfr.zod.src/locales/sw/index.ts (2)
3-13: LGTM: Import syntax standardized.The JSON imports correctly use
with { type: 'json' }, replacing the deprecatedassertsyntax. This change aligns with the ECMAScript standard.
15-27: No remainingzodusage in sw locale Verified absence of any.zodreferences across all TS/TSX files; consumers have migrated away.
| "@tanstack/react-query-devtools": "5.90.2", | ||
| "@tanstack/react-router": "1.132.47", | ||
| "@tanstack/react-router-devtools": "1.132.51", | ||
| "@tanstack/react-start": "1.132.51", | ||
| "@tanstack/zod-adapter": "1.132.47", |
There was a problem hiding this comment.
Align TanStack router packages to the same patch version.
@tanstack/react-start@1.132.51 and @tanstack/react-router-devtools@1.132.51 will each pull in @tanstack/react-router@1.132.51, while the root dependency is pinned to 1.132.47. PNPM will therefore install two copies of @tanstack/react-router, splitting router singletons (context, store, etc.) and causing runtime breakage. Please bump the root @tanstack/react-router to 1.132.51 (or downgrade the others) so the stack dedupes to one build.
🤖 Prompt for AI Agents
In package.json around lines 63 to 67, the @tanstack/react-router root
dependency is pinned to 1.132.47 while other TanStack packages in the same block
are at 1.132.51, which will cause PNPM to install two copies of
@tanstack/react-router and split router singletons; update the root
"@tanstack/react-router" version to "1.132.51" (or alternatively downgrade the
other two to 1.132.47) so all TanStack router-related packages use the same
patch version and dedupe to a single build.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
src/server/routers/book.ts (4)
98-98: Duplicate: ID validation consistency concern.As flagged in the previous review, this should potentially use
.cuid()if the database enforces CUID format for IDs.
24-24: Duplicate: Cursor validation weakened.As flagged in the previous review, removing
.cuid()validation allows any string format for cursor, which could cause database errors if the ID column enforces CUID format.
33-33: Duplicate: Output cursor validation weakened.Same cursor validation issue as input cursor.
165-165: Duplicate: ID validation inconsistency in updateById.As flagged in the previous review, the
idfield should use.cuid()for consistency with the database ID format.src/lib/zod/zod-utils.ts (2)
17-24: Fix type mismatch in params default value.Same issue as
required: the default value is a string but should be an object or undefined.Apply this diff:
- nullable: ( - params: Parameters<typeof z.string>[0] = t('common:errors.required') - ) => + nullable: (params?: Parameters<typeof z.string>[0]) => z .string(params) .transform(emptyStringAsNull) .nullable() .pipe(z.string(params).nullable()),
33-40: Fix type mismatch in params default value.Same issue as the other methods.
Apply this diff:
- optional: ( - params: Parameters<typeof z.string>[0] = t('common:errors.required') - ) => + optional: (params?: Parameters<typeof z.string>[0]) => z .string(params) .transform(emptyStringAsUndefined) .optional() .pipe(z.string(params).optional()),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/layout/manager/nav-sidebar.tsx(1 hunks)src/lib/zod/zod-utils.ts(1 hunks)src/server/routers/book.ts(7 hunks)src/server/routers/genre.ts(2 hunks)src/server/routers/user.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/server/routers/user.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-09T08:29:28.532Z
Learnt from: ivan-dalmet
PR: BearStudio/start-ui-web#620
File: src/components/form/field-number/docs.stories.tsx:23-23
Timestamp: 2025-10-09T08:29:28.532Z
Learning: In Zod v4, the unified `error` parameter is used for custom error messages. For example, `z.number({ error: 'Required' })` is the correct syntax, replacing the older Zod v3 pattern of `required_error` and `invalid_type_error`.
Applied to files:
src/lib/zod/zod-utils.ts
🧬 Code graph analysis (2)
src/server/routers/genre.ts (1)
src/features/genre/schema.ts (1)
zGenre(8-15)
src/server/routers/book.ts (1)
src/features/book/schema.ts (2)
zBook(10-19)zFormFieldsBook(22-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Playwright E2E Tests
- GitHub Check: 🔬 Tests (24)
- GitHub Check: 🔬 Tests (22)
🔇 Additional comments (6)
src/server/routers/genre.ts (3)
37-37: LGTM! Log message corrected.Good catch fixing the log message from "Getting books from database" to "Getting genres from database".
46-59: LGTM! Parallel execution improves performance.Migrating from sequential/transactional execution to
Promise.allparallelization is a solid performance improvement. Both operations are read-only (countandfindMany), making parallelization safe with no risk of race conditions or consistency issues.
23-23: Verify cursor validation relaxation is intentionalThe
cursorinsrc/server/routers/genre.ts(and similarly in other routers) now usesz.string().optional(), accepting any string. Since your Prisma IDs are defined with@default(cuid()), confirm you intended to drop.cuid()validation or if you should reinstatez.string().cuid().optional().src/layout/manager/nav-sidebar.tsx (1)
1-1: ‘use no memo’ directive is correct: Verified per React Compiler docs that"use no memo"is the proper syntax to opt out of compiler optimizations in React 19+.src/server/routers/book.ts (2)
57-71: Approve transaction to Promise.all refactor for read-only queries.The change from
$transactiontoPromise.allis appropriate here since both operations are read-only.Promise.allallows concurrent execution and may improve performance without sacrificing correctness.
25-28: Invalid Zod method.prefault()will cause runtime errors.Zod does not have a
.prefault()method. The correct method is.default(). This will cause immediate errors when the schema is evaluated.Apply this diff to fix all occurrences:
- limit: z.coerce.number().int().min(1).max(100).prefault(20), - searchTerm: z.string().trim().optional().prefault(''), + limit: z.coerce.number().int().min(1).max(100).default(20), + searchTerm: z.string().trim().optional().default(''), }) - .prefault({}) + .default({})⛔ Skipped due to learnings
Learnt from: ivan-dalmet PR: BearStudio/start-ui-web#620 File: src/env/server.ts:13-14 Timestamp: 2025-10-09T08:30:25.898Z Learning: In Zod v4, `prefault()` is a valid method that acts as a "pre-parse default". It differs from `default()` by applying the fallback value before the inner schema runs, whereas `default()` short-circuits parsing. Usage like `z.coerce.number().int().prefault(2592000)` is correct and ensures the default value goes through the validation chain.
| required: ( | ||
| params: Parameters<typeof z.string>[0] = t('common:errors.required') | ||
| ) => z.string(params).transform(emptyStringAsNull).pipe(z.string(params)), |
There was a problem hiding this comment.
Fix type mismatch in params default value.
The default value t('common:errors.required') is a string, but Parameters<typeof z.string>[0] expects an object like { error?: string } in Zod v4. This mismatch will cause type errors.
Based on learnings
Apply this diff to fix the type:
- required: (
- params: Parameters<typeof z.string>[0] = t('common:errors.required')
- ) => z.string(params).transform(emptyStringAsNull).pipe(z.string(params)),
+ required: (params?: Parameters<typeof z.string>[0]) =>
+ z.string(params).transform(emptyStringAsNull).pipe(z.string(params)),📝 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.
| required: ( | |
| params: Parameters<typeof z.string>[0] = t('common:errors.required') | |
| ) => z.string(params).transform(emptyStringAsNull).pipe(z.string(params)), | |
| required: (params?: Parameters<typeof z.string>[0]) => | |
| z.string(params).transform(emptyStringAsNull).pipe(z.string(params)), |
🤖 Prompt for AI Agents
In src/lib/zod/zod-utils.ts around lines 14 to 16, the default value for params
is a plain string but Parameters<typeof z.string>[0] expects an object; replace
the string default with an object that matches Zod's string params shape (e.g. {
message: t('common:errors.required') }) so the call becomes z.string(params)
with params typed correctly, and update any imports/types if necessary to ensure
the object shape satisfies Zod v4's expected parameter type.
| nullish: ( | ||
| params: Parameters<typeof z.string>[0] = t('common:errors.required') | ||
| ) => | ||
| z | ||
| .string(params) | ||
| .transform(emptyStringAsNull) | ||
| .nullish() | ||
| .pipe(z.string(params).nullish()), |
There was a problem hiding this comment.
Fix type mismatch in params default value.
Same issue as required and nullable.
Apply this diff:
- nullish: (
- params: Parameters<typeof z.string>[0] = t('common:errors.required')
- ) =>
+ nullish: (params?: Parameters<typeof z.string>[0]) =>
z
.string(params)
.transform(emptyStringAsNull)
.nullish()
.pipe(z.string(params).nullish()),📝 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.
| nullish: ( | |
| params: Parameters<typeof z.string>[0] = t('common:errors.required') | |
| ) => | |
| z | |
| .string(params) | |
| .transform(emptyStringAsNull) | |
| .nullish() | |
| .pipe(z.string(params).nullish()), | |
| nullish: (params?: Parameters<typeof z.string>[0]) => | |
| z | |
| .string(params) | |
| .transform(emptyStringAsNull) | |
| .nullish() | |
| .pipe(z.string(params).nullish()), |
🤖 Prompt for AI Agents
In src/lib/zod/zod-utils.ts around lines 25 to 32, the default value for the
params argument (t('common:errors.required')) has a type mismatch with the
declared type; change the default to match the other helpers (use undefined as
the default) so the params signature aligns with Parameters<typeof z.string>[0]
and pass that through to z.string(params) as the other functions do.
| cursor: z.string().cuid().optional(), | ||
| limit: z.coerce.number().int().min(1).max(100).default(20), | ||
| cursor: z.string().optional(), | ||
| limit: z.coerce.number().int().min(1).max(100).prefault(20), |
There was a problem hiding this comment.
🧩 Analysis chain
Verify .prefault() is a valid Zod 4 method.
The migration from .default() to .prefault() is consistent across the codebase, but since my knowledge of Zod 4 is limited (it was released after my training cutoff), please confirm that .prefault() is the correct Zod 4 method for setting default values.
Also applies to: 27-27
🌐 Web query:
What is the Zod 4 method for setting default values? Is it .prefault() or something else?
💡 Result:
Use .default(). Zod v4 uses z.string().default(value) (the default short-circuits parsing). The term "prefault" described the old Zod‑3 behavior; to replicate that old behavior in v4 you can use a transform/pipe (e.g. z.transform(v => v ?? default).pipe(T)). [1][2]
Sources:
[1] Zod v4 docs — Defaults.
[2] Zod v4 changelog / GitHub discussion explaining "prefault" and how to replicate it.
Replace .prefault() with .default()
Zod v4 removed .prefault(); use .default(value) for defaults (and add a transform pipeline if you need pre‐parse default behavior).
Applies to src/server/routers/genre.ts lines 24 and 27
🤖 Prompt for AI Agents
In src/server/routers/genre.ts around lines 24 and 27 the schema uses the
removed Zod v4 method `.prefault()`; replace `.prefault(20)` (and the other
`.prefault(...)` on line 27) with `.default(20)` (or the desired default value),
and if you require a default that is applied before parsing use a transform
pipeline or `z.preprocess()` to set the value prior to validation.
ivan-dalmet
force-pushed
the
v3/update-deps
branch
2 times, most recently
from
646b41e to
605fd58
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/server/auth.tsx (1)
94-109: Extract demo-mode guard and improve error messageBoth
databaseHooks.user.create.beforeand.update.beforeshare identical demo-mode checks. Consider:const guardDemoMode = async <T>(data: T): Promise<{ data: T }> => { if (envClient.VITE_IS_DEMO) { throw new Error('User modifications are disabled in demo mode'); } return { data }; }; databaseHooks: { user: { create: { before: guardDemoMode }, update: { before: guardDemoMode }, }, },This removes duplication and provides a clearer error message.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.example(1 hunks)src/env/server.ts(2 hunks)src/server/auth.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-09T08:30:25.898Z
Learnt from: ivan-dalmet
PR: BearStudio/start-ui-web#620
File: src/env/server.ts:13-14
Timestamp: 2025-10-09T08:30:25.898Z
Learning: In Zod v4, `prefault()` is a valid method that acts as a "pre-parse default". It differs from `default()` by applying the fallback value before the inner schema runs, whereas `default()` short-circuits parsing. Usage like `z.coerce.number().int().prefault(2592000)` is correct and ensures the default value goes through the validation chain.
Applied to files:
src/env/server.ts
🧬 Code graph analysis (1)
src/server/auth.tsx (2)
src/env/server.ts (1)
envServer(9-40)src/env/client.ts (1)
envClient(24-52)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 23-23: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🔬 Tests (lts/*)
- GitHub Check: Playwright E2E Tests
- GitHub Check: 🔬 Tests (24)
🔇 Additional comments (7)
.env.example (2)
24-26: LGTM! Environment variable renaming is consistent.The renaming of session-related variables and the change from
AUTH_TRUSTED_ORIGINtoAUTH_TRUSTED_ORIGINS(plural) aligns correctly with the updated parsing logic insrc/env/server.tsthat splits comma-separated values.
23-23: Quote the placeholder value to ensure proper parsing.The value
REPLACE MEcontains spaces and should be quoted. Some dotenv parsers may fail or misinterpret unquoted values with spaces.Apply this diff:
-AUTH_SECRET="REPLACE ME" # You can use `npx @better-auth/cli@latest secret` to a generated secret +AUTH_SECRET="REPLACE ME" # You can use `npx @better-auth/cli@latest secret` to generate a secretNote: This issue also applies to lines 29-30 (
GITHUB_CLIENT_IDandGITHUB_CLIENT_SECRET).Based on static analysis hints.
Likely an incorrect or invalid review comment.
src/server/auth.tsx (1)
25-28: LGTM! Session and trusted origins configuration updated correctly.The session configuration now uses the renamed environment variables (
AUTH_SESSION_EXPIRATION_IN_SECONDSandAUTH_SESSION_UPDATE_AGE_IN_SECONDS), andtrustedOriginscorrectly referencesAUTH_TRUSTED_ORIGINS, which is pre-parsed as an array insrc/env/server.ts.src/env/server.ts (4)
13-21: LGTM! Session and trusted origins configuration is well-designed.The session configuration with
prefault()ensures default values go through the validation chain, and theAUTH_TRUSTED_ORIGINSparsing logic correctly handles comma-separated values with proper trimming.Based on learnings.
26-26: LGTM! Email server URL validation is consistent.The
EMAIL_SERVERconfiguration correctly uses thez.url()shorthand, consistent with theDATABASE_URLpattern.
42-55: Confirm correct use oferrorkey in Zod v4 refine. This replaces the deprecatedmessageoption; no changes needed.
11-12: Approve environment schema update
Zod v4.1.12 supports thez.url()shorthand and the newAUTH_SECRET: z.string()is valid.
| .optional() | ||
| .refine( | ||
| (value) => | ||
| // Check in prodution if the value is not REPLACE ME |
There was a problem hiding this comment.
Fix typo in comment.
The comment has a typo: "prodution" should be "production".
Apply this diff:
- // Check in prodution if the value is not REPLACE ME
+ // Check in production if the value is not REPLACE ME📝 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.
| // Check in prodution if the value is not REPLACE ME | |
| // Check in production if the value is not REPLACE ME |
🤖 Prompt for AI Agents
In src/env/server.ts at line 48, fix the typo in the comment: change "prodution"
to "production" so the comment reads "Check in production if the value is not
REPLACE ME". Make only this correction in the comment text.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/layout/manager/nav-sidebar.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: 🔬 Tests (lts/*)
- GitHub Check: 🔬 Tests (24)
| <Link to="/manager/dashboard"> | ||
| {({ isActive }) => ( | ||
| <SidebarMenuButton asChild isActive={isActive}> | ||
| <span> | ||
| <LayoutDashboardIcon /> | ||
| <span>{t('layout:nav.dashboard')}</span> | ||
| </span> | ||
| </SidebarMenuButton> | ||
| )} | ||
| </Link> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the asChild + <span> pattern for semantics and accessibility.
The current pattern uses asChild on SidebarMenuButton with a <span> child, which causes the button to render as a non-interactive span element. When TanStack Router's Link adds click handlers to this span, it creates an accessibility issue—spans are not keyboard-navigable or recognizable as interactive elements by assistive technologies.
Additionally, this pattern is inconsistent with the logo link (lines 41-47), where SidebarMenuButton asChild directly wraps the Link component.
Consider one of these approaches:
- Remove
asChildand letSidebarMenuButtonrender its own interactive element:
<Link to="/manager/dashboard">
{({ isActive }) => (
- <SidebarMenuButton asChild isActive={isActive}>
- <span>
- <LayoutDashboardIcon />
- <span>{t('layout:nav.dashboard')}</span>
- </span>
+ <SidebarMenuButton isActive={isActive}>
+ <LayoutDashboardIcon />
+ <span>{t('layout:nav.dashboard')}</span>
</SidebarMenuButton>
)}
</Link>- Align with the logo pattern by making Link the child of SidebarMenuButton (if render props aren't essential):
-<Link to="/manager/dashboard">
- {({ isActive }) => (
- <SidebarMenuButton asChild isActive={isActive}>
- <span>
+<SidebarMenuButton asChild>
+ <Link to="/manager/dashboard">
+ <span>
<LayoutDashboardIcon />
<span>{t('layout:nav.dashboard')}</span>
- </span>
- </SidebarMenuButton>
- )}
-</Link>
+ </span>
+ </Link>
+</SidebarMenuButton>Note: Option 2 loses access to the isActive render prop, so you'd need an alternative way to determine active state.
Also applies to: 79-88, 107-116
🏁 Script executed:
#!/bin/bash
# Locate the SidebarMenuButton component definition
rg -n "SidebarMenuButton" -g "src/**/*.tsx"Length of output: 1526
🏁 Script executed:
#!/bin/bash
# Show SidebarMenuButton implementation
sed -n '480,620p' src/components/ui/sidebar.tsxLength of output: 5102
🏁 Script executed:
#!/bin/bash
# Show import section of nav-sidebar.tsx
sed -n '1,40p' src/layout/manager/nav-sidebar.tsxLength of output: 1015
Ensure menu items render interactive elements. Using SidebarMenuButton asChild with a <span> produces a non-focusable element that receives click handlers—breaking keyboard and assistive‐tech navigation. Remove asChild so it renders a native <button>, or slot the <Link> inside SidebarMenuButton so an anchor/button element receives the event handlers.
🤖 Prompt for AI Agents
In src/layout/manager/nav-sidebar.tsx around lines 67 to 76, the current pattern
uses SidebarMenuButton with asChild wrapping a plain <span>, which yields a
non-focusable element and breaks keyboard/assistive navigation; either remove
the asChild prop so SidebarMenuButton renders a native <button> directly, or
move the <Link> inside SidebarMenuButton (so the underlying rendered
anchor/button receives the event handlers and is focusable). Update the JSX
accordingly so the interactive element is an actual button/anchor (not a span)
and retains accessibility and keyboard support.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e-tests.yml(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.28.0)
.github/workflows/e2e-tests.yml
[high] 22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 🔬 Tests (24)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: Playwright E2E Tests
- GitHub Check: 🔬 Tests (lts/*)
| DATABASE_URL: postgres://startui:startui@localhost:5432/startui | ||
| VITE_IS_DEMO: false | ||
| BETTER_AUTH_SECRET: fyXjLxjXJowjicI2BAjxOaUsQd1QafdegZDciJE/xM8= | ||
| AUTH_SECRET: fyXjLxjXJowjicI2BAjxOaUsQd1QafdegZDciJE/xM8= |
There was a problem hiding this comment.
Remove hardcoded auth secret from workflow.
A real AUTH_SECRET is committed directly in the workflow. This leaks credentials to anyone cloning the repo or viewing workflow logs. Pull it from secrets (e.g. AUTH_SECRET: ${{ secrets.AUTH_SECRET }}) and revoke/rotate the exposed value immediately.
🧰 Tools
🪛 Gitleaks (8.28.0)
[high] 22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
.github/workflows/e2e-tests.yml around line 22 contains a hardcoded AUTH_SECRET
value; remove the literal secret from the file, replace it with a reference to
the repository secret (e.g. set AUTH_SECRET to use GitHub Actions secrets like
${ { secrets.AUTH_SECRET } }), add the real secret into the repository/org
GitHub Secrets UI, and immediately revoke/rotate the exposed credential so the
leaked value is invalidated.
Summary by CodeRabbit
New Features
Bug Fixes / UX
Refactor
Chores
Tests
Localization cleanup