-
-
Notifications
You must be signed in to change notification settings - Fork 39
refactor: migrate to zod 4 #368
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
base: main
Are you sure you want to change the base?
Conversation
- Introduced a new parser module to manage file context and configuration. - Updated VeliteFile creation to accept loaders directly. - Adjusted schema imports to utilize the new parser for accessing current file and configuration. - Enabled file nesting in VSCode settings for improved project organization.
- Introduced a new parser module to manage file context and configuration. - Updated VeliteFile creation to accept loaders directly. - Adjusted schema imports to utilize the new parser for accessing current file and configuration. - Enabled file nesting in VSCode settings for improved project organization.
- Introduced a new parser module to manage file context and configuration. - Updated VeliteFile creation to accept loaders directly. - Adjusted schema imports to utilize the new parser for accessing current file and configuration. - Enabled file nesting in VSCode settings for improved project organization.
- Introduced a new parser module to manage file context and configuration. - Updated VeliteFile creation to accept loaders directly. - Adjusted schema imports to utilize the new parser for accessing current file and configuration. - Enabled file nesting in VSCode settings for improved project organization.
- Added @velite/plugin-next as a dependency in the example's package.json. - Refactored next.config.ts to utilize withVelite for configuration management. - Updated pnpm-lock.yaml to include workspace references for the new plugin.
|
Warning Rate limit exceeded@zce has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughMigrates from a custom Zod v3 fork to Zod v4 and introduces an AsyncLocalStorage-based parser context (context(), parseWithContext). Refactors schema transforms to use context(), replaces meta-based file access, adds a Next.js plugin package, updates examples/dependencies, and removes many forked zod internals. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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 |
- Introduced a new parser module to manage file context and configuration. - Updated VeliteFile creation to accept loaders directly. - Adjusted schema imports to utilize the new parser for accessing current file and configuration. - Enabled file nesting in VSCode settings for improved project organization.
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: 7
🧹 Nitpick comments (5)
test/basic.ts (1)
2-2: Update or remove misleading comment.The comment says "import from source" but the actual behavior is now the opposite - the test imports nothing from source and instead runs the built distribution via
npm run build. Consider updating this comment to reflect the actual behavior, such as "test built distribution" or removing it entirely.Apply this diff to clarify the comment:
- // import from source + // test built distributionsrc/context.ts (1)
35-37: Parametercontextshadows the exported function name.The parameter name
contextshadows the exportedcontextfunction, which could cause confusion if someone tries to callcontext()inside the callback or if this function is extended later.-export const parseWithContext = async (schema: Schema, data: unknown, context: Context) => { - return store.run(context, () => schema.safeParseAsync(data)) +export const parseWithContext = async (schema: Schema, data: unknown, ctx: Context) => { + return store.run(ctx, () => schema.safeParseAsync(data)) }examples/nextjs/velite.config.ts (1)
23-35: Consider extracting thetimestamp()helper to reduce duplication.This helper is duplicated between
examples/vite/velite.config.tsandexamples/nextjs/velite.config.ts. If this is a commonly needed pattern, consider providing it as a built-in schema (e.g.,s.gitTimestamp()) or documenting it in a shared location.packages/next/index.js (1)
42-44: Consider adding error handling forbuild()failures.If
velite.build()throws, the error will propagate and potentially crash the Next.js startup without a clear indication of what went wrong. Wrapping in try-catch with a descriptive error message would improve developer experience.- const { build } = await import('velite') - - await build({ ...pluginOptions, watch: isDev, clean: !isDev }) + try { + const { build } = await import('velite') + await build({ ...pluginOptions, watch: isDev, clean: !isDev }) + } catch (err) { + console.error('[velite] Build failed:', err) + throw err + }src/build.ts (1)
54-58: Type cast(issue as any).fatalloses type safety.Zod v4's
ZodIssuetype doesn't include afatalproperty by default. The customfatalflag is passed throughctx.addIssue()but requires a type assertion to access. Consider defining a custom issue type or extending the Zod types for better type safety:interface VeliteIssue extends z.ZodIssue { fatal?: boolean }This is a minor concern that doesn't affect runtime behavior.
📜 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 (43)
.vscode/settings.json(1 hunks)examples/basic/velite.config.js(5 hunks)examples/nextjs/next.config.ts(1 hunks)examples/nextjs/package.json(1 hunks)examples/nextjs/velite.config.ts(2 hunks)examples/vite/package.json(1 hunks)examples/vite/velite.config.ts(2 hunks)package.json(1 hunks)packages/next/index.d.ts(1 hunks)packages/next/index.js(1 hunks)packages/next/package.json(1 hunks)packages/next/readme.md(1 hunks)src/build.ts(4 hunks)src/config.ts(0 hunks)src/context.ts(1 hunks)src/file.ts(2 hunks)src/index.ts(1 hunks)src/schemas/excerpt.ts(2 hunks)src/schemas/file.ts(2 hunks)src/schemas/image.ts(3 hunks)src/schemas/index.ts(2 hunks)src/schemas/isodate.ts(1 hunks)src/schemas/markdown.ts(3 hunks)src/schemas/mdx.ts(4 hunks)src/schemas/metadata.ts(2 hunks)src/schemas/path.ts(2 hunks)src/schemas/raw.ts(1 hunks)src/schemas/slug.ts(1 hunks)src/schemas/toc.ts(2 hunks)src/schemas/unique.ts(1 hunks)src/schemas/zod/ZodError.ts(0 hunks)src/schemas/zod/errors.ts(0 hunks)src/schemas/zod/external.ts(0 hunks)src/schemas/zod/helpers/enumUtil.ts(0 hunks)src/schemas/zod/helpers/errorUtil.ts(0 hunks)src/schemas/zod/helpers/parseUtil.ts(0 hunks)src/schemas/zod/helpers/partialUtil.ts(0 hunks)src/schemas/zod/helpers/typeAliases.ts(0 hunks)src/schemas/zod/helpers/util.ts(0 hunks)src/schemas/zod/index.ts(0 hunks)src/schemas/zod/locales/en.ts(0 hunks)src/types.ts(2 hunks)test/basic.ts(1 hunks)
💤 Files with no reviewable changes (12)
- src/schemas/zod/index.ts
- src/schemas/zod/external.ts
- src/schemas/zod/helpers/typeAliases.ts
- src/schemas/zod/helpers/errorUtil.ts
- src/schemas/zod/locales/en.ts
- src/schemas/zod/errors.ts
- src/schemas/zod/ZodError.ts
- src/schemas/zod/helpers/partialUtil.ts
- src/schemas/zod/helpers/enumUtil.ts
- src/schemas/zod/helpers/parseUtil.ts
- src/config.ts
- src/schemas/zod/helpers/util.ts
🧰 Additional context used
🧬 Code graph analysis (23)
packages/next/index.js (2)
packages/next/index.d.ts (2)
createNextPlugin(19-19)withVelite(19-19)src/build.ts (1)
build(231-268)
test/basic.ts (2)
test/vite.ts (2)
t(6-32)res(8-8)test/nextjs.ts (1)
res(8-8)
src/context.ts (5)
src/types.ts (1)
Config(283-304)src/file.ts (1)
VeliteFile(15-88)src/schemas/index.ts (1)
Schema(32-32)src/schemas/zod/helpers/parseUtil.ts (2)
ParseContext(53-65)ZodMeta(40-40)src/schemas/zod/types.ts (4)
safeParseAsync(254-277)_parse(4052-4188)_parse(4509-4556)safeParse(224-246)
src/schemas/isodate.ts (1)
src/schemas/zod/types.ts (2)
ZodString(614-1168)ZodStringDef(524-528)
src/schemas/mdx.ts (4)
src/context.ts (1)
context(26-30)src/schemas/file.ts (1)
file(17-28)src/schemas/zod/helpers/parseUtil.ts (1)
addIssueToContext(74-88)src/assets.ts (1)
options(169-207)
examples/vite/velite.config.ts (2)
examples/basic/velite.config.js (1)
execAsync(24-24)src/context.ts (1)
context(26-30)
src/schemas/raw.ts (1)
src/context.ts (1)
context(26-30)
src/schemas/slug.ts (1)
src/schemas/unique.ts (1)
unique(32-40)
packages/next/index.d.ts (1)
packages/next/index.js (4)
createNextPlugin(10-48)createNextPlugin(10-48)withVelite(50-50)withVelite(50-50)
src/schemas/path.ts (3)
src/context.ts (1)
context(26-30)src/schemas/file.ts (1)
file(17-28)src/schemas/zod/types.ts (2)
path(4066-4068)RefinementCtx(37-41)
src/schemas/excerpt.ts (2)
src/context.ts (1)
context(26-30)src/schemas/zod/helpers/parseUtil.ts (1)
addIssueToContext(74-88)
src/schemas/unique.ts (2)
src/schemas/path.ts (1)
path(26-39)src/context.ts (1)
context(26-30)
src/schemas/markdown.ts (2)
src/context.ts (1)
context(26-30)src/schemas/file.ts (1)
file(17-28)
src/schemas/toc.ts (2)
src/context.ts (1)
context(26-30)src/schemas/file.ts (1)
file(17-28)
package.json (1)
src/config.ts (1)
path(39-67)
examples/basic/velite.config.js (2)
src/context.ts (1)
context(26-30)src/schemas/index.ts (1)
s(16-30)
src/file.ts (2)
src/loaders/index.ts (1)
loaders(5-5)src/types.ts (1)
Loader(88-99)
examples/nextjs/next.config.ts (2)
packages/next/index.d.ts (1)
withVelite(19-19)packages/next/index.js (2)
withVelite(50-50)withVelite(50-50)
src/schemas/metadata.ts (3)
src/context.ts (1)
context(26-30)src/schemas/zod/types.ts (3)
ctx(302-306)RefinementCtx(37-41)arg(4058-4065)src/schemas/zod/helpers/parseUtil.ts (1)
addIssueToContext(74-88)
examples/nextjs/velite.config.ts (1)
src/context.ts (1)
context(26-30)
examples/vite/package.json (1)
test/vite.ts (1)
t(6-32)
src/schemas/index.ts (1)
src/schemas/zod/types.ts (3)
deepPartialify(2238-2264)ZodObject(2266-2716)ZodArray(2056-2176)
src/build.ts (3)
src/schemas/file.ts (1)
file(17-28)src/context.ts (1)
parseWithContext(35-37)src/schemas/unique.ts (1)
uniqueCache(25-25)
🔇 Additional comments (46)
.vscode/settings.json (1)
8-8: Nice developer ergonomics improvement for the refactoring work.Enabling file nesting in the explorer will help visualize related files (e.g.,
.test.tsalongside source files) during this large-scale migration, making navigation easier. This is a helpful addition for the team's local development experience.packages/next/package.json (1)
17-20: Peer dependency range vs actual Next supportThe manifest looks fine. Please confirm that the plugin is tested and supported starting from
next@16.0.0; if you still intend to support Next 15 as well, consider widening thenextpeer range accordingly.package.json (1)
60-65: Dependency bumps and Zod 4 adoptionThe dependency updates (esbuild, terser, chokidar, @types/node) and addition of
zod@^4.1.13look consistent with the Zod 4 migration and Node 18+ target. Please make sure you run the full build/test suite (including any file‑watch scenarios) to catch regressions fromchokidar@5and the newer bundler/minifier versions.Also applies to: 70-72
examples/vite/package.json (1)
13-14: React/Vite toolchain version refreshThe version bumps for React 19.2.3, Vite 7.2.x, TypeScript, and the ESLint stack look coherent for the Vite example. Please verify that
pnpm build(and any example‑specific tests liketest/vite.ts) still pass, since generated.veliteoutputs and lint rules can be sensitive to these upgrades.Also applies to: 17-32
examples/nextjs/package.json (1)
13-15: Next.js example dependency updates and plugin wiringAdding
@velite/plugin-nextand aligning Next/React/Tailwind/TS versions looks consistent with the new Next plugin integration. Please confirm that the Next.js example (pnpm dev/pnpm buildandvelite --clean) still works end‑to‑end with these versions and the new plugin.Also applies to: 18-19, 22-27
src/index.ts (1)
1-7: Central re-exports includingcontextlook goodThe additional re-exports (
context,logger,schemas,types) make the public surface more discoverable and align with the new context-based parsing model. Just be aware this cementscontextas part of the public API going forward.src/schemas/excerpt.ts (1)
1-3: Zod 4custom+ context-based excerpt transform looks correctThe migration to
zod’scustomplus the new(value, ctx)transform,context().file.plainfallback, andctx.addIssue({ code: 'custom', ... })usage all look compatible with Zod 4 and the new AsyncLocalStorage-backed context. The logic preserves previous behavior (optional frontmatter override, error on empty content) while dropping the old meta plumbing.Also applies to: 21-25
src/schemas/image.ts (1)
3-3: Image schema migration to Zod 4 context-aware transform looks solidThe switch to
zod.string().transform<Image>(async (value, ctx) => ...)withcontext()for relative assets andctx.addIssue({ fatal: true, code: 'custom', message })on failure is consistent with Zod 4’s transform/issue model and the new global context API. Absolute-path handling viaabsoluteRootremains intact, and missing parser context is now reported as a proper Zod issue instead of leaking an uncaught error.Also applies to: 6-7, 27-27, 46-47, 49-53
examples/basic/velite.config.js (4)
5-5: LGTM! Context import added correctly.The addition of
contextimport aligns with the new context-based approach for accessing file and config information throughout the transforms.
25-34: LGTM! Timestamp transform migrated correctly to context-based approach.The transform now uses
context().file.pathinstead of the previous meta-based access, which aligns with the migration to Zod v4 and the new AsyncLocalStorage-based context system.
66-66: LGTM! Simplified slug() calls align with updated API.The removal of the explicit
'global'argument is correct since it's now the default value for thegroupparameter in the updatedslug()function signature.Also applies to: 79-79, 92-92
96-96: LGTM! Page transform migrated correctly to context-based approach.The use of
context().file.basenameis consistent with the broader migration from meta-based to context-based file information access.src/schemas/isodate.ts (1)
1-6: LGTM! Clean migration to official Zod v4 package.The import source change from the local
'./zod'module to the official'zod'package is correct. The function logic remains unchanged, which is appropriate for this migration.src/schemas/raw.ts (1)
1-8: LGTM! Raw schema migrated correctly to context-based approach.The changes properly migrate to:
- Official Zod v4 package import
- Context-based file content access via
context().file.content- Simplified transform with appropriate fallback handling
test/basic.ts (1)
8-9: LGTM! Test now validates built distribution.The change from importing and calling
build()directly to executingnpm run buildis a better testing approach, as it validates the actual packaged output that users would consume.src/schemas/markdown.ts (4)
1-11: LGTM! Imports updated correctly for Zod v4 migration.The changes properly import from:
- Official Zod v4 package (
'zod')- New context system (
'../context')
42-51: LGTM! Transform migrated correctly to context-based approach.The transform callback has been properly updated to:
- Use
ctxparameter instead of destructured{ meta, addIssue }- Access file and config via
context()- Use
ctx.addIssue()for error reporting- Source content from
file.contentinstead ofmeta.content
77-77: LGTM! File path correctly sourced from context.Using
file.pathfrom the context-based system is consistent with the migration pattern.
79-81: LGTM! Error handling updated correctly.The catch block now uses
ctx.addIssue()with the appropriate parameters, includingfatal: trueto halt parsing on error.src/schemas/slug.ts (1)
1-14: LGTM! Cleaner slug implementation with centralized uniqueness checking.The refactor simplifies the slug schema by:
- Delegating uniqueness checking to the
unique()schema- Renaming the parameter from
bytogroupfor clarity- Removing custom duplicate-checking logic
Note: The parameter rename from
bytogroupis a breaking change, but it's appropriately documented in the JSDoc and the default value remains'global'.examples/nextjs/next.config.ts (1)
1-3: LGTM! Simplified Next.js integration with new Velite plugin.The migration to the
@velite/plugin-nextpackage provides a much cleaner integration pattern compared to the previous manual webpack plugin approach. This is a significant improvement in developer experience.src/schemas/file.ts (1)
1-4: LGTM! Imports updated correctly for Zod v4 migration.The imports have been properly updated to use the official Zod v4 package and the new context system.
src/context.ts (1)
10-30: Clean context implementation using AsyncLocalStorage.The
Contextinterface withreadonlyfields ensures immutability, and thecontext()function provides clear error messaging when called outside the parsing pipeline. This is a solid foundation for the Zod 4 migration.examples/vite/velite.config.ts (1)
4-4: Import ofcontextaligns with the new context-based parsing approach.The migration from
metatocontext()for accessing file information is consistent with the broader refactor in this PR.src/schemas/index.ts (2)
1-2: Clean migration to official Zod v4.Replacing the custom Zod fork with the official
zodpackage simplifies maintenance and provides access to Zod v4's new features.
16-32: Schema bundle preserves backward compatibility while adding custom schemas.Spreading
zintosallows users to continue usings.string(),s.object(), etc., while also accessing Velite-specific schemas likes.image()ands.toc(). Theas constassertion is appropriate here.src/schemas/toc.ts (2)
5-7: Clean migration to context-based file access.The switch from meta-based access to
context()for retrieving file content aligns with the PR's Zod 4 migration strategy.
116-119: Error handling follows consistent pattern.Using
ctx.addIssuewithfatal: trueand returningnull as neveris consistent with other schema transformers in this codebase (e.g.,src/schemas/file.tslines 24-26).src/schemas/path.ts (2)
2-4: LGTM! Clean migration to Zod v4 and new context system.The import changes correctly transition from the internal Zod fork to the official
zodpackage, and thecontextimport aligns with the new AsyncLocalStorage-based context mechanism.
27-34: LGTM! Context-based path resolution is correctly implemented.The transformer signature change from
(value, { meta, addIssue })to(value, ctx)withcontext()for accessingconfigandfileis consistent with the pattern used in other schema files (e.g.,src/schemas/file.ts). Thectx.addIssuecall follows Zod v4 conventions.src/schemas/mdx.ts (3)
3-6: LGTM! Imports correctly updated for Zod v4 migration.The transition to importing
customfromzodandcontextfrom../contextaligns with the broader refactoring pattern across the schema layer.
22-30: LGTM! Context-based content and config resolution is well implemented.The transformer correctly:
- Uses the new
(value, ctx)signature- Defaults
valuetofile.contentwhen not provided- Reports issues via
ctx.addIssuefollowing Zod v4 conventions- Destructures
mdxandoutputfrom the context-providedconfig
54-70: LGTM! Compilation and error handling properly updated.The
compilecall correctly usesfile.pathfrom context, and error handling appropriately usesctx.addIssuewithfatal: trueto signal unrecoverable errors.packages/next/index.js (3)
1-3: LGTM! Attribution noted.Good practice to credit the inspiration source.
19-26: Complex but necessary mode detection for Next.js v16+.The
isDevdetection logic handles the Next.js v16+ architecture wherenext devspawns a forked process (next-start) that doesn't inheritprocess.argv. The combination ofcommand === undefinedandNODE_ENV === 'development'is a reasonable heuristic.Consider adding a brief inline comment explaining why both conditions are needed together.
50-50: LGTM! Convenient default export.Pre-configured
withVeliteinstance allows zero-config usage for common cases.src/types.ts (2)
209-214: LGTM! Clear naming for hook context type.Renaming from
ContexttoHookContextprovides clearer semantics and avoids potential confusion with the new parser context system insrc/context.ts.
270-277: LGTM! Hook signatures updated consistently.Both
prepareandcompletehooks now correctly referenceHookContext, maintaining API consistency.packages/next/index.d.ts (2)
1-7: LGTM! Type definitions properly structured.The imports and internal types correctly support both Next.js config objects and config functions, which is important for compatibility with different Next.js configuration patterns.
19-19: LGTM! Clean public API exports.The exports correctly expose
createNextPluginfor custom configuration,withVelitefor default usage, and theOptionstype for TypeScript consumers.src/schemas/metadata.ts (2)
1-3: LGTM! Clean migration to Zod v4 with context-based approach.The import changes correctly migrate from the custom Zod fork to the official
zodpackage, and the newcontext()import properly replaces the metadata-driven file access pattern.
65-70: LGTM! Transform callback correctly adapted to Zod v4 API.The callback signature
(value, ctx)and usage ofctx.addIssue()align with Zod v4's transform refinement context. The fallback tocontext().file.plainproperly replaces the previousmeta.plainaccess pattern.src/schemas/unique.ts (2)
5-23: LGTM! UniqueCache implementation is sound.The cache correctly stores entries with composite keys and provides a reset mechanism. The iteration-while-deleting pattern in
reset(path)is safe in JavaScript Maps.Consider that the O(n) iteration in
reset(path)could become a performance concern with very large caches, but this is acceptable for typical use cases.
32-40: LGTM! Async refinement correctly adapted to Zod v4.The
superRefinewith async callback is valid in Zod v4. The fatal flag ensures duplicate values halt validation appropriately. Thecontext().file.pathaccess for cache storage integrates well with the AsyncLocalStorage-based context system.src/build.ts (2)
8-16: LGTM! Import changes align with the new context-based architecture.The imports correctly bring in
parseWithContextfor the new parsing flow anduniqueCachefor cross-call deduplication. The type import simplification removes the now-unnecessaryZodMeta.
176-184: LGTM! Cache invalidation strategy is well-designed.Full cache reset on config changes and path-specific reset on file changes provide the right granularity for incremental rebuilds. This cleanly replaces the previous manual cache eviction approach.
| .transform<string>(async (value, { addIssue }) => { | ||
| if (value != null) { | ||
| addIssue({ fatal: false, code: 'custom', message: '`s.timestamp()` schema will resolve the value from `git log -1 --format=%cd`' }) | ||
| } | ||
| const { stdout } = await execAsync(`git log -1 --format=%cd ${meta.path}`) | ||
| const { stdout } = await execAsync(`git log -1 --format=%cd ${context().file.path}`) | ||
| return new Date(stdout || Date.now()).toISOString() | ||
| }) |
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.
Same command injection risk as in the Vite example.
This timestamp() helper has the same vulnerability where the file path is passed directly to a shell command without escaping.
Apply the same fix using execFile instead of exec:
+import { execFile } from 'node:child_process'
+
+const execFileAsync = promisify(execFile)
+
const timestamp = () =>
s
.custom<string | undefined>(i => i === undefined || typeof i === 'string')
.transform<string>(async (value, { addIssue }) => {
if (value != null) {
addIssue({ fatal: false, code: 'custom', message: '`s.timestamp()` schema will resolve the value from `git log -1 --format=%cd`' })
}
- const { stdout } = await execAsync(`git log -1 --format=%cd ${context().file.path}`)
+ const { stdout } = await execFileAsync('git', ['log', '-1', '--format=%cd', context().file.path])
return new Date(stdout || Date.now()).toISOString()
})Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In examples/nextjs/velite.config.ts around lines 29 to 35, the transform uses
execAsync with a file path interpolated into a shell command causing a command
injection risk; replace this with a safe execFile-based call (or promisified
execFile) that passes the git and format args plus context().file.path as an
argument (do not interpolate into a shell string), await the execFile result,
and return the ISO timestamp from stdout (falling back to Date.now() if stdout
is empty); ensure any error is handled similarly to current behavior and that
addIssue logic remains unchanged.
| .transform<string>(async (value, { addIssue }) => { | ||
| if (value != null) { | ||
| addIssue({ fatal: false, code: 'custom', message: '`s.timestamp()` schema will resolve the value from `git log -1 --format=%cd`' }) | ||
| } | ||
| const { stdout } = await execAsync(`git log -1 --format=%cd ${meta.path}`) | ||
| const { stdout } = await execAsync(`git log -1 --format=%cd ${context().file.path}`) | ||
| return new Date(stdout || Date.now()).toISOString() | ||
| }) |
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.
Potential command injection via unescaped file path.
The file path is interpolated directly into a shell command without escaping. If a file path contains shell metacharacters (spaces, quotes, $, ;, etc.), it could cause command failures or injection vulnerabilities.
Consider escaping the path or using execFile which doesn't invoke a shell:
+import { execFile } from 'node:child_process'
+
+const execFileAsync = promisify(execFile)
+
const timestamp = () =>
s
.custom<string | undefined>(i => i === undefined || typeof i === 'string')
.transform<string>(async (value, { addIssue }) => {
if (value != null) {
addIssue({ fatal: false, code: 'custom', message: '`s.timestamp()` schema will resolve the value from `git log -1 --format=%cd`' })
}
- const { stdout } = await execAsync(`git log -1 --format=%cd ${context().file.path}`)
+ const { stdout } = await execFileAsync('git', ['log', '-1', '--format=%cd', context().file.path])
return new Date(stdout || Date.now()).toISOString()
})Committable suggestion skipped: line range outside the PR's diff.
| declare const createNextPlugin: (options?: Options) => <T extends NextConfigInput>(nextConfig?: T) => Promise<T> | ||
|
|
||
| /** | ||
| * Next.js plugin for integrating Velite | ||
| */ | ||
| declare const withVelite: ReturnType<typeof createNextPlugin> |
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.
🧩 Analysis chain
🏁 Script executed:
cat packages/next/index.js | head -80Repository: zce/velite
Length of output: 1673
🏁 Script executed:
cat packages/next/index.d.tsRepository: zce/velite
Length of output: 722
Correct the type signature to reflect actual behavior.
The generic <T extends NextConfigInput> with return type Promise<T> creates a type mismatch when T is NextConfigFunction. The implementation returns nextConfig as-is without invoking it, which means if a function-style config is passed, the return type Promise<NextConfigFunction> represents a Promise resolving to a function—not a valid Next.js config. Either restrict the generic to exclude NextConfigFunction, or invoke the function before returning to ensure the resolved value is always a plain NextConfig object.
🤖 Prompt for AI Agents
In packages/next/index.d.ts around lines 12 to 17, the exported createNextPlugin
signature allows T to be NextConfigFunction but returns Promise<T>, which would
make the Promise resolve to a function (invalid as a Next.js config); fix by
narrowing the generic so T excludes NextConfigFunction (e.g. T extends
NextConfigInput but not a function) or change the implementation/signature to
detect and call a function-style nextConfig and return Promise<NextConfig> (i.e.
always resolve to a plain NextConfig object) — pick one approach and update the
type declaration accordingly so the declared return type matches actual runtime
behavior.
| # @velite/plugin-next | ||
|
|
||
| A Next.js plugin for integrating Velite content processing. | ||
|
|
||
| ## Installation | ||
|
|
||
| ```bash | ||
| npm install -D velite @velite/plugin-next | ||
| ``` | ||
|
|
||
| ## Usage | ||
|
|
||
| ```ts | ||
| // next.config.ts | ||
| import { withVelite } from '@velite/plugin-next' | ||
|
|
||
| export default withVelite({ | ||
| // other next config here... | ||
| }) | ||
| ``` | ||
|
|
||
| ## License | ||
|
|
||
| [MIT](../../license) © [zce](https://zce.me) |
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.
Verify license link path/casing
README content looks good. Please double-check that ../../license matches the actual filename in the repo (often LICENSE), otherwise this link may 404 on case‑sensitive paths.
🤖 Prompt for AI Agents
In packages/next/readme.md around lines 1 to 24, the README links to
../../license which may not match the repository filename/casing (commonly
LICENSE) and can 404 on case-sensitive filesystems; update the link to the exact
filename and path used in the repo (for example ../../LICENSE) or use an
absolute/root-relative link to the repo license, then verify the link works in a
case-sensitive environment.
| static async create(path: string, loaders: Loader[]): Promise<VeliteFile> { | ||
| const loader = loaders.find(l => l.test.test(path)) | ||
| const file = new VeliteFile({ path }) | ||
| if (loader == null) return file.fail(`no loader found for '${path}'`) | ||
| file.value = await readFile(path) | ||
| file.data = await loader.load(file) | ||
| if (file.data?.data == null) return file.fail(`no data loaded from '${path}'`) | ||
| loaded.set(path, file) | ||
| return 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.
🧩 Analysis chain
🏁 Script executed:
rg -n -A10 'VeliteFile.create' src/build.tsRepository: zce/velite
Length of output: 490
🏁 Script executed:
ast-grep --pattern $'get records() {
$$$
}'Repository: zce/velite
Length of output: 36
🏁 Script executed:
rg -n 'fail\(' src/file.ts -A2Repository: zce/velite
Length of output: 349
🏁 Script executed:
rg -n 'records' src/file.tsRepository: zce/velite
Length of output: 120
🏁 Script executed:
cat -n src/file.ts | head -100Repository: zce/velite
Length of output: 3048
Add error checking after VeliteFile.create() before accessing file.records.
When fail() is called on lines 81 or 84, the file is returned without a valid data property set. In src/build.ts, file.records is accessed on line 41 without verifying the file is valid. Since records returns this.data.data (line 24), accessing it on a failed file returns undefined. Add a check for file.records == null or check for VFile errors before processing the returned file.
🤖 Prompt for AI Agents
In src/build.ts around line 41, after calling VeliteFile.create(...) check
whether the returned file has valid records (e.g., if file.records == null or
file.hasErrors()) before proceeding; if records are missing, handle the error
path (log/propagate/skip) and do not access file.records. Update the code to
short-circuit on a failed/invalid VeliteFile to avoid accessing undefined data
and ensure downstream logic only runs when file.records is present.
| export const file = ({ allowNonRelativePath = true }: FileOptions = {}) => | ||
| string().transform<string>(async (value, { meta, addIssue }) => { | ||
| string().transform<string>(async (value, ctx) => { | ||
| try { | ||
| if (allowNonRelativePath && !isRelativePath(value)) return value | ||
| const { output } = meta.config | ||
| return await processAsset(value, meta.path, output.name, output.base) | ||
| const { file, config } = context() | ||
| return await processAsset(value, file.path, config.output.name, config.output.base) | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : String(err) | ||
| addIssue({ fatal: true, code: 'custom', message }) | ||
| ctx.addIssue({ fatal: true, code: 'custom', message, continue: false }) | ||
| return null as never | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the file in context
find . -name "file.ts" -path "*/schemas/*" | head -5Repository: zce/velite
Length of output: 75
🌐 Web query:
Zod v4 ctx.addIssue() API documentation continue parameter
💡 Result:
Short answer: There is no "continue" parameter on ctx.addIssue in Zod v4. addIssue does not stop validation by default (it "continues"); to stop/abort further refinements you must set fatal: true on the issue and return z.NEVER from the refinement. Example:
ts
const schema = z.number().superRefine((val, ctx) => {
if (val < 10) {
ctx.addIssue({ code: z.ZodIssueCode.custom, message: "too small", fatal: true });
return z.NEVER;
}
});
Docs show the fatal pattern; see the Zod docs and the related GitHub issue discussing v4 behavior. [1][2]
Sources:
[1] Zod docs — superRefine / ctx.addIssue (abort early with fatal + z.NEVER).
[2] GitHub issue discussing v4 change where ctx.addIssue continues by default.
🌐 Web query:
Zod v4 ZodIssue options parameters type definition
💡 Result:
Short answer — in Zod v4 the "issue" shape is a union of core $ZodIssue variants (each with a code, path, and message plus variant-specific fields). The canonical definitions live in the core package (types like $ZodIssueInvalidType, $ZodIssueTooBig, $ZodIssueTooSmall, $ZodIssueInvalidStringFormat, $ZodIssueNotMultipleOf, $ZodIssueUnrecognizedKeys, $ZodIssueInvalidValue, $ZodIssueInvalidUnion, $ZodIssueCustom, etc.). Each variant includes:
- code: ZodIssueCode (string literal discriminator)
- path: (string | number)[]
- message: string
- variant-specific props (e.g. expected/received for invalid_type; minimum/maximum/inclusive for too_small/too_big; params or other metadata on some issues).
See Zod v4 changelog/type list and the errors docs for the exact TypeScript interfaces and full union. [1][2]
Sources:
[1] Zod v4 changelog — updated issue formats (lists $ZodIssue* variants).
[2] Zod errors/types doc (example ZodIssue interface and specific issue fields).
🏁 Script executed:
# Read the actual file to see the full context
cat -n ./src/schemas/file.ts | head -40Repository: zce/velite
Length of output: 1198
Remove the invalid continue: false parameter from ctx.addIssue().
Zod v4's ctx.addIssue() does not support a continue parameter. This parameter is not part of the Zod v4 API and should be removed. To properly abort validation after reporting an issue in Zod v4, use fatal: true (which is already set correctly) and return z.NEVER instead of null as never.
Change line 25 from:
ctx.addIssue({ fatal: true, code: 'custom', message, continue: false })to:
ctx.addIssue({ fatal: true, code: 'custom', message })Optionally, also update the return value to z.NEVER for consistency with Zod v4 patterns.
🤖 Prompt for AI Agents
In src/schemas/file.ts around lines 17 to 27, remove the invalid continue: false
property from ctx.addIssue() and update the function to return z.NEVER instead
of null as never; specifically call ctx.addIssue({ fatal: true, code: 'custom',
message }) and return z.NEVER (ensure z is imported from 'zod' at the top if not
already).
| async (value, ctx) => { | ||
| const { file } = context() | ||
| value = value ?? file.content | ||
| if (value == null || value.length === 0) { | ||
| addIssue({ code: 'custom', message: 'The content is empty' }) | ||
| ctx.addIssue({ code: 'custom', message: 'The content is empty' }) | ||
| return (options?.original ? {} : []) as T extends { original: true } ? TocTree : TocEntry[] | ||
| } | ||
| try { | ||
| // extract ast tree from markdown/mdx content | ||
| const tree = value != null ? fromMarkdown(value) : meta.mdast | ||
| const tree = value != null ? fromMarkdown(value) : file.mdast | ||
| if (tree == null) throw new Error('No tree found') |
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.
Unreachable else branch in tree parsing logic.
After line 105, value is assigned value ?? file.content. At line 106, if value is null/empty, the function returns early. Therefore, at line 111, value is guaranteed to be a non-empty string, making the ternary condition value != null always true and file.mdast unreachable.
Simplify to always parse the value:
try {
- const tree = value != null ? fromMarkdown(value) : file.mdast
+ const tree = fromMarkdown(value)
if (tree == null) throw new Error('No tree found')🤖 Prompt for AI Agents
In src/schemas/toc.ts around lines 103 to 112, the ternary using value != null ?
fromMarkdown(value) : file.mdast is unreachable because value was already set to
value ?? file.content and the function returned earlier if value was null/empty;
replace the ternary with a direct parse of the confirmed non-null string (e.g.,
const tree = fromMarkdown(value)) and remove the unreachable branch, keeping the
existing try/catch and error handling so behavior and types remain the same.
- Introduced a new parser module to manage file context and configuration. - Updated VeliteFile creation to accept loaders directly. - Adjusted schema imports to utilize the new parser for accessing current file and configuration. - Enabled file nesting in VSCode settings for improved project organization.
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: 0
🧹 Nitpick comments (1)
docs/other/snippets.md (1)
1-295: Clarify context() import in code examples.The examples use
context()throughout but don't show its import. Consider adding an import statement (e.g.,import { context } from 'velite') in the first few examples, or a brief note explaining that context() is available in Velite schema transforms. This will improve clarity for readers new to the API.For the timestamp schema example, consider adding the import:
```ts import { stat } from 'fs/promises' import { defineSchema } from 'velite' +import { context } from 'velite' const timestamp = defineSchema(() =>Or add a note before the first example explaining that
context()and related utilities are available in Velite schema transforms.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/guide/custom-schema.md(5 hunks)docs/guide/define-collections.md(1 hunks)docs/guide/last-modified.md(2 hunks)docs/guide/using-mdx.md(2 hunks)docs/guide/velite-schemas.md(1 hunks)docs/other/snippets.md(5 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- docs/guide/custom-schema.md
- docs/guide/define-collections.md
- docs/guide/velite-schemas.md
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (7)
docs/guide/using-mdx.md (1)
315-349: Documentation properly reflects the new context-based transform API.The examples correctly demonstrate the new AsyncLocalStorage-based context pattern: imports
contextfrom velite, accesses file/config viacontext()in transforms, and reports errors viactx.addIssue. The transformation flow and error handling are clearly shown.Please verify that the
contextfunction is properly exported from the velite module's public API to support these documented examples.docs/guide/last-modified.md (2)
15-15: File stat example properly demonstrates the new context-based API.The first example correctly shows the updated pattern: importing
context, accessing file path viacontext().file.pathin the transform handler, and error reporting viactx.addIssue. The transformation is clear and well-integrated.Also applies to: 20-20, 25-26
49-49: Git timestamp example consistently demonstrates the context-based approach.The second example mirrors the first, properly showing
contextimport, file path access within the transform, and consistent error handling. Both examples form a cohesive demonstration of the new API.Verify that the
contextfunction is exported from the velite module and that the transform callback signature shown in these examples matches the implementation in the codebase (likelysrc/context.tsbased on the PR scope).Also applies to: 56-56, 60-61
docs/other/snippets.md (4)
14-20: Example correctly demonstrates the context() API pattern.The transform callback signature and error handling properly reflect the Zod v4 migration. The shift from meta-based to context()-driven file access is clear and consistent.
47-52: Git timestamp example correctly uses context() for file access.The updated pattern consistently applies the new context() API and ctx.addIssue() error handling.
78-88: Remote image transform correctly applies the new ctx.addIssue() pattern.Error handling is properly updated to use the context parameter. The example appropriately omits context() since remote image operations don't depend on file metadata.
278-291: Excerpt example correctly demonstrates multi-value context() destructuring.The example shows how to access both file and config from context(), and properly uses file.content as a fallback. Error handling with ctx.addIssue() is consistent throughout.
- Introduced a new parser module to manage file context and configuration. - Updated VeliteFile creation to accept loaders directly. - Adjusted schema imports to utilize the new parser for accessing current file and configuration. - Enabled file nesting in VSCode settings for improved project organization.
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.
Pull request overview
This PR migrates Velite from a custom fork of Zod 3 to the official Zod 4 package, introducing a centralized context API for accessing file and config information during schema parsing.
Key Changes:
- Migrates from custom Zod fork to official Zod 4.1.13
- Introduces
AsyncLocalStorage-based context API for accessing file/config during parsing - Replaces
metaparameter withcontext()function in custom schemas - Updates all built-in schemas and examples to use the new context API
Reviewed changes
Copilot reviewed 49 out of 52 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/context.ts | New context API using AsyncLocalStorage for file/config access |
| src/build.ts | Updated to use parseWithContext and new unique cache management |
| src/file.ts | Removed config coupling from VeliteFile constructor |
| src/schemas/unique.ts | New UniqueCache class with context-based validation |
| src/schemas/*.ts | All schema files migrated to use Zod 4 and context() API |
| src/schemas/zod/* | Removed custom Zod fork (4800+ lines deleted) |
| src/types.ts | Renamed Context to HookContext, removed cache from Config |
| package.json | Added zod@4.1.13 dependency, version bumped to 1.0.0-alpha.1 |
| examples/* | Updated to use context() function instead of meta parameter |
| docs/* | Documentation updated to reflect new context API |
| packages/next/* | New Next.js plugin package added |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
test/basic.ts:1
- This line is very long and hard to read. Consider breaking it into multiple lines or extracting the exec callback into a named function for better readability.
src/schemas/unique.ts:1 - The error message format has changed from the original which was more descriptive. Consider including the current file path in the message to make it clearer where the duplicate was detected, e.g.,
Duplicate value '${value}' in '${context().file.path}', already exists in '${conflict}'
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const context = () => { | ||
| const ctx = store.getStore() | ||
| if (ctx) return ctx | ||
| throw new Error('Missing parser context') |
Copilot
AI
Dec 15, 2025
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.
The error message 'Missing parser context' is not very helpful for debugging. Consider adding information about when this error might occur or how to fix it, such as 'Context is only available during schema parsing. Ensure you are calling context() within a schema transform or validation function.'
| throw new Error('Missing parser context') | |
| throw new Error("Missing parser context: Context is only available during schema parsing. Ensure you are calling context() within a schema transform or validation function.") |
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : String(err) | ||
| addIssue({ fatal: true, code: 'custom', message }) | ||
| ctx.addIssue({ fatal: true, code: 'custom', message, continue: false }) |
Copilot
AI
Dec 15, 2025
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.
The continue property is not a standard Zod issue property. In Zod 4, fatal controls whether processing should stop. The continue: false is redundant and should be removed.
| ctx.addIssue({ fatal: true, code: 'custom', message, continue: false }) | |
| ctx.addIssue({ fatal: true, code: 'custom', message }) |
| result.error.issues.forEach(issue => { | ||
| const source = [...pathPrefix, ...(issue.path ?? [])].map(p => (typeof p === 'number' ? `[${p}]` : String(p))).join('.') | ||
| const message = file.message(issue.message ?? 'Validation error', { source }) | ||
| message.fatal = (issue as any).fatal || config.strict === true |
Copilot
AI
Dec 15, 2025
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.
The type assertion (issue as any).fatal suggests the issue type might be incomplete. Consider defining a proper type extension for issues with a fatal property or document why this property exists outside the standard Zod issue type.
closes #341
Summary by CodeRabbit
New Features
Improvements
Configuration
✏️ Tip: You can customize this high-level summary in your review settings.