feat: export Zod schemas and publish JSON Schema#5
Conversation
- Export jsonMapTransformSchema, jsonMapDynamicSchema, jsonMapMapSchema, and jsonMapOptionsSchema from src/types.ts - Derive TypeScript types via z.infer<> (schema-first) - Re-export schemas from src/index.ts - Add zod-to-json-schema dev dependency and generation script - Generate jsonmap.schema.json at repo root on every build - Add schema validation tests in src/types.test.ts - Include jsonmap.schema.json in published package files Closes #4
Summary of ChangesHello @karmaniverous, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's type safety and interoperability by adopting a schema-first approach using Zod. It makes the internal data structures more explicit and consumable by external tools through the generation and publication of a JSON Schema. This change improves the developer experience by providing clearer type definitions and enabling automated validation, without altering the core functionality of the JsonMap itself. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR implements the export of Zod schemas and generation of JSON Schema for the JsonMap library, addressing issue #4. The changes follow a schema-first approach where Zod schemas become the source of truth and TypeScript types are derived from them.
Changes:
- Exported Zod validation schemas (jsonMapTransformSchema, jsonMapDynamicSchema, jsonMapMapSchema, jsonMapOptionsSchema) from src/types.ts with comprehensive JSDoc documentation
- Added comprehensive test coverage for all exported schemas in src/types.test.ts
- Created a build-time script to generate jsonmap.schema.json from the Zod schemas using zod-to-json-schema
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Converted to schema-first pattern with exported Zod schemas and types derived via z.infer, added comprehensive documentation |
| src/types.test.ts | New test file with validation tests for all exported schemas |
| src/index.ts | Added exports for both Zod schemas and TypeScript types |
| scripts/generate-json-schema.ts | New script to generate JSON Schema from Zod schema at build time |
| package.json | Added zod-to-json-schema dev dependency, integrated schema generation into build process, included schema file in published package |
| package-lock.json | Dependency tree updates for new zod-to-json-schema package |
| jsonmap.schema.json | Generated JSON Schema file for JsonMapMap type |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "typescript": "^5.5.3", | ||
| "typescript-eslint": "^7.16.1" | ||
| "typescript-eslint": "^7.16.1", | ||
| "zod-to-json-schema": "^3.25.1" |
There was a problem hiding this comment.
The zod-to-json-schema package requires zod ^3.25 or ^4 as a peer dependency, but the existing zod dependency in package.json is ^3.23.8, which does not satisfy this requirement. This will cause a peer dependency warning and potentially runtime issues. The zod dependency should be updated to at least ^3.25.0 in the same PR that adds zod-to-json-schema.
| name: 'JsonMapMap', | ||
| }); | ||
|
|
||
| const outPath = resolve(import.meta.dirname ?? '.', '..', 'jsonmap.schema.json'); |
There was a problem hiding this comment.
The fallback value '.' for import.meta.dirname will resolve relative to the current working directory, not the script directory. If the script is run from a different directory, it will write the schema file to the wrong location. Consider using import.meta.url with fileURLToPath instead for better compatibility, or use __dirname if running with ts-node. For example: const __dirname = fileURLToPath(new URL('.', import.meta.url));
| "scripts": { | ||
| "build": "rimraf dist && rollup --config rollup.config.ts --configPlugin @rollup/plugin-typescript", | ||
| "build": "rimraf dist && rollup --config rollup.config.ts --configPlugin @rollup/plugin-typescript && npm run generate:schema", | ||
| "generate:schema": "node --experimental-strip-types scripts/generate-json-schema.ts", |
There was a problem hiding this comment.
The generate:schema script uses the --experimental-strip-types flag, which requires Node.js version 22.6.0 or higher. This is not documented in the package.json engines field, which could lead to confusing errors for users or CI systems running older Node versions. Consider adding an engines field to package.json specifying the minimum Node version, or use a more compatible approach like ts-node which is already a dev dependency.
| * Type definitions and Zod schemas for JsonMap. | ||
| * | ||
| * @remarks | ||
| * All Zod schemas are the source of truth; TypeScript types are derived via {@link z.infer}. |
There was a problem hiding this comment.
The JSDoc link tag references z.infer, which is a generic type utility from Zod, not a linkable entity in the codebase. This will likely not generate a working link in documentation tools. Consider removing the @link tag and using backticks instead, like: "TypeScript types are derived via z.infer<>"
| * All Zod schemas are the source of truth; TypeScript types are derived via {@link z.infer}. | |
| * All Zod schemas are the source of truth; TypeScript types are derived via `z.infer<>`. |
| */ | ||
| export const jsonMapOptionsSchema = z.object({ | ||
| ignore: z.union([z.string(), z.instanceof(RegExp)]).optional(), |
There was a problem hiding this comment.
The z.instanceof(RegExp) schema cannot be reliably serialized to JSON Schema, as JSON Schema has no native RegExp type. While this is not a problem for the current JSON Schema generation (which only exports jsonMapMapSchema), it may cause issues if consumers try to generate JSON Schema from jsonMapOptionsSchema in the future. Consider documenting this limitation or providing an alternative validation approach for JSON Schema consumers.
| */ | |
| export const jsonMapOptionsSchema = z.object({ | |
| ignore: z.union([z.string(), z.instanceof(RegExp)]).optional(), | |
| * | |
| * @remarks | |
| * The {@link RegExp} branch is validated via a custom predicate and cannot be | |
| * faithfully represented in standard JSON Schema. JSON Schema consumers should | |
| * treat this field as accepting strings only, or handle RegExp values via | |
| * out-of-band conventions. | |
| */ | |
| export const jsonMapOptionsSchema = z.object({ | |
| ignore: z | |
| .union([ | |
| z.string(), | |
| // Use a custom validator instead of z.instanceof(RegExp) to avoid | |
| // JSON Schema serialization issues. | |
| z.custom<RegExp>((val) => val instanceof RegExp, 'RegExp'), | |
| ]) | |
| .optional(), |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great addition, establishing Zod schemas as the single source of truth and publishing a JSON schema for consumers. The changes are well-structured, including a generation script, build integration, and tests.
I have a few suggestions to improve the maintainability and robustness of the implementation:
- In
src/types.ts, I've suggested a change to better align the recursiveJsonMapMaptype with its Zod schema, improving type safety. - In
package.json, I recommend usingts-nodefor the schema generation script to avoid an experimental Node.js flag. - In the generation script itself, I've proposed a way to create a more readable and robust JSON schema by providing named definitions.
Overall, this is a solid feature, and these changes should make it even better.
| "scripts": { | ||
| "build": "rimraf dist && rollup --config rollup.config.ts --configPlugin @rollup/plugin-typescript", | ||
| "build": "rimraf dist && rollup --config rollup.config.ts --configPlugin @rollup/plugin-typescript && npm run generate:schema", | ||
| "generate:schema": "node --experimental-strip-types scripts/generate-json-schema.ts", |
There was a problem hiding this comment.
The JSDoc in scripts/generate-json-schema.ts suggests running the script with ts-node, which is already a dev dependency. Using ts-node is generally more robust than relying on an experimental Node.js flag. Consider aligning the script with the documentation for consistency and to avoid experimental features in the build process.
| "generate:schema": "node --experimental-strip-types scripts/generate-json-schema.ts", | |
| "generate:schema": "ts-node --esm scripts/generate-json-schema.ts", |
| import { jsonMapMapSchema } from '../src/types.ts'; | ||
|
|
||
| const jsonSchema = zodToJsonSchema(jsonMapMapSchema, { | ||
| name: 'JsonMapMap', | ||
| }); |
There was a problem hiding this comment.
The generated JSON schema contains a very long and fragile $ref pointer: #/definitions/JsonMapMap/anyOf/1/additionalProperties/anyOf/1/properties/$/anyOf/0. This could easily break if the Zod schema structure changes.
You can produce a more robust and readable JSON schema by providing the other schemas as named definitions to zodToJsonSchema. This will encourage the generator to use named $refs (e.g., #/definitions/jsonMapTransform) instead of long JSON pointers.
| import { jsonMapMapSchema } from '../src/types.ts'; | |
| const jsonSchema = zodToJsonSchema(jsonMapMapSchema, { | |
| name: 'JsonMapMap', | |
| }); | |
| import { | |
| jsonMapDynamicSchema, | |
| jsonMapMapSchema, | |
| jsonMapTransformSchema, | |
| } from '../src/types.ts'; | |
| const jsonSchema = zodToJsonSchema(jsonMapMapSchema, { | |
| name: 'JsonMapMap', | |
| definitions: { | |
| jsonMapTransform: jsonMapTransformSchema, | |
| jsonMapDynamic: jsonMapDynamicSchema, | |
| }, | |
| }); |
| /** | ||
| * Schema for a recursive JsonMap map definition. | ||
| * | ||
| * @remarks | ||
| * A map node is either a JSON literal, an array of map nodes, or an object | ||
| * whose values are map nodes or {@link JsonMapDynamic} nodes. | ||
| */ | ||
| export const jsonMapMapSchema: z.ZodType< | ||
| Literal | Record<string, unknown> | unknown[] | ||
| > = z.lazy(() => | ||
| z.union([ | ||
| literalSchema, | ||
| z.record(z.union([jsonMapMapSchema, jsonMapDynamicSchema])), | ||
| z.array(jsonMapMapSchema), | ||
| ]), | ||
| ); | ||
|
|
||
| /** | ||
| * A recursive JsonMap map definition. | ||
| * | ||
| * @remarks | ||
| * This type is the union of JSON literals, objects mapping string keys to | ||
| * either nested maps or {@link JsonMapDynamic} nodes, and arrays of maps. | ||
| */ | ||
| export type JsonMapMap = | ||
| | Literal | ||
| | { [key: string]: JsonMapMap | JsonMapDynamic } | ||
| | JsonMapMap[]; |
There was a problem hiding this comment.
This is a great refactoring to adopt Zod schemas as the source of truth.
To improve maintainability and type safety for the recursive JsonMapMap type, I'd suggest a couple of adjustments:
- Define the
JsonMapMaptype before thejsonMapMapSchema. - Use
JsonMapMapas the explicit type forjsonMapMapSchema.
This makes the relationship between the type and the schema explicit and ensures they are consistent. While the type is still manually defined (a necessity for complex recursive types like this), this change makes it clear that jsonMapMapSchema is the runtime validator for the JsonMapMap type. It also makes the code slightly easier to follow.
/**
* A recursive JsonMap map definition.
*
* @remarks
* This type is the union of JSON literals, objects mapping string keys to
* either nested maps or {@link JsonMapDynamic} nodes, and arrays of maps.
*/
export type JsonMapMap =
| Literal
| { [key: string]: JsonMapMap | JsonMapDynamic }
| JsonMapMap[];
/**
* Schema for a recursive JsonMap map definition.
*
* @remarks
* A map node is either a JSON literal, an array of map nodes, or an object
* whose values are map nodes or {@link JsonMapDynamic} nodes.
*/
export const jsonMapMapSchema: z.ZodType<JsonMapMap> = z.lazy(() =>
z.union([
literalSchema,
z.record(z.union([jsonMapMapSchema, jsonMapDynamicSchema])),
z.array(jsonMapMapSchema),
]),
);- Update zod from ^3.23.8 to ^4.0.0 - Remove zod-to-json-schema dev dependency (replaced by native z.toJSONSchema) - Update z.record() calls to use explicit key type (Zod 4 breaking change) - Remove redundant .required() calls on object schemas - Update generate-json-schema script to use z.toJSONSchema() Part of #4
Major version updates: - eslint 8→9, typescript-eslint 7→8, @eslint/js 9 (latest) - eslint-plugin-mocha 10→11 (flat config export changed) - eslint-config-prettier 9→10, eslint-plugin-tsdoc 0.2→0.5 - chai 5→6, mocha 10→11, @types/chai 4→5 - cross-env 7→10, rimraf 5→6, lefthook 1→2 - release-it 17→19 - @rollup/plugin-alias 5→6, commonjs 26→29, node-resolve 15→16, typescript 11→12 Config fixes: - eslint.config.js: mochaPlugin.configs.flat.recommended → .configs.recommended (v11 API) - eslint.config.js: disable no-unused-expressions in test files (Chai compat) - .mocharc.json: narrow spec to ./src/**/*.test.ts (mocha 11 glob change) - Remove @types/eslint-plugin-mocha (v11 ships own types) Held back: @eslint/js and eslint capped at ^9 (eslint 10 lacks ecosystem support)
- Remove mocha, chai, nyc, cross-env, source-map-support, jsdom-global, ts-node - Remove eslint-plugin-mocha, @types/mocha, @types/chai, @types/eslint-plugin-mocha, @types/eslint__js, @types/eslint-config-prettier - Add vitest with config excluding .rollup.cache - Rewrite all tests using vitest describe/it/expect - Update test script: vitest run - Remove .mocharc.json - Update eslint.config.js (remove mocha plugin)
ESLint: - Convert eslint.config.js → eslint.config.ts (type-aware flat config) - Add @vitest/eslint-plugin + eslint-plugin-prettier - Use strictTypeChecked rules from typescript-eslint - Lint target changed to 'eslint .' (flat config auto-discovery) STAN: - Add stan.config.yml with typecheck, lint, test, build, knip scripts - All scripts passing (lint has 1 acceptable tsdoc warning) Cleanup: - Remove unused src/util/packageName.ts (flagged by knip) - Remove unused devDeps: @rollup/plugin-terser, auto-changelog, fs-extra - Enable allowImportingTsExtensions in tsconfig - Fix rollup.config.ts unused import - Fix generate-json-schema.ts unnecessary conditional
…xpressions) Both rules removed from eslint.config.ts — no code depended on them. stan run all green, zero warnings.
Build fix: - Remove allowImportingTsExtensions from tsconfig (conflicts with rollup plugin) - Switch generate:schema from node --experimental-strip-types to tsx - Use .js extension in script import (tsx resolves to .ts) README improvements: - Add Quick Start section - Document path syntax with reference table - Document progressive transformations - Document private key behavior and ignore option - Document recursive evaluation - Add API reference section - Add exported types table - Fix stale unit test link - Update JSON Schema ref to Draft 2020-12
- Update .vscode/extensions.json and settings.json to match npm-package-template-ts - Add cross-env, use NO_COLOR=1 in build script for cleaner stan output - Remove mocha test explorer config (replaced by vitest explorer)
tsconfig.json: - Clean up commented template → lean config matching npm-package-template-ts - Add skipLibCheck, vitest/globals types, target ES2022 rollup.config.ts: - Add outputToFilesystem: false to prevent outDir validation errors - Add include filter to prevent transient rollup.config-*.mjs artifacts - Set incremental: false, noEmit: false overrides for bundling - Fixes build error on fresh clones .gitignore: - Sync with template (add local/, context/, granular .stan/ entries) - Remove tracked .stan/diff/ files
|
/gemini review |
1 similar comment
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces Zod schemas as the source of truth for types and adds a build step to generate a JSON Schema file. It also migrates the project from Mocha to Vitest and updates the ESLint configuration to the flat config format. While the architectural direction is sound, there are several critical issues: the PR references multiple non-existent package versions (e.g., Zod 4, TypeScript 5.9, Vitest 4) and uses a non-existent z.toJSONSchema method. Additionally, the ESLint configuration logic for rule extraction is brittle and should be refactored to use standard patterns.
| "lodash": "^4.17.23", | ||
| "nanoid": "^5.1.6", | ||
| "zod": "^4.3.6" | ||
| }, | ||
| "description": "A hyper-generic JSON mapping library.", | ||
| "devDependencies": { | ||
| "@eslint/js": "^9.7.0", | ||
| "@rollup/plugin-alias": "^5.1.0", | ||
| "@rollup/plugin-commonjs": "^26.0.1", | ||
| "@eslint/js": "^9.22.0", | ||
| "@rollup/plugin-alias": "^6.0.0", | ||
| "@rollup/plugin-commonjs": "^29.0.0", | ||
| "@rollup/plugin-json": "^6.1.0", | ||
| "@rollup/plugin-node-resolve": "^15.2.3", | ||
| "@rollup/plugin-terser": "^0.4.4", | ||
| "@rollup/plugin-typescript": "^11.1.6", | ||
| "@types/chai": "^4.3.16", | ||
| "@types/eslint__js": "^8.42.3", | ||
| "@types/eslint-config-prettier": "^6.11.3", | ||
| "@types/eslint-plugin-mocha": "^10.4.0", | ||
| "@types/lodash": "^4.17.7", | ||
| "@types/mocha": "^10.0.7", | ||
| "@rollup/plugin-node-resolve": "^16.0.3", | ||
| "@rollup/plugin-typescript": "^12.3.0", | ||
| "@types/lodash": "^4.17.23", | ||
| "@types/numeral": "^2.0.5", | ||
| "auto-changelog": "^2.4.0", | ||
| "chai": "^5.1.1", | ||
| "cross-env": "^7.0.3", | ||
| "eslint": "^8.57.0", | ||
| "eslint-config-prettier": "^9.1.0", | ||
| "eslint-plugin-mocha": "^10.4.3", | ||
| "@vitest/eslint-plugin": "^1.6.9", | ||
| "cross-env": "^10.1.0", | ||
| "eslint": "^9.22.0", | ||
| "eslint-config-prettier": "^10.1.8", | ||
| "eslint-plugin-prettier": "^5.5.5", | ||
| "eslint-plugin-simple-import-sort": "^12.1.1", | ||
| "eslint-plugin-tsdoc": "^0.2.17", | ||
| "fs-extra": "^11.2.0", | ||
| "jsdom-global": "^3.0.2", | ||
| "knip": "^5.26.0", | ||
| "lefthook": "^1.7.2", | ||
| "mocha": "^10.6.0", | ||
| "eslint-plugin-tsdoc": "^0.5.0", | ||
| "jiti": "^2.6.1", | ||
| "knip": "^5.84.1", | ||
| "lefthook": "^2.1.1", | ||
| "numeral": "^2.0.6", | ||
| "nyc": "^17.0.0", | ||
| "prettier": "^3.3.3", | ||
| "release-it": "^17.6.0", | ||
| "rimraf": "^5.0.9", | ||
| "rollup": "^4.18.1", | ||
| "rollup-plugin-dts": "^6.1.1", | ||
| "source-map-support": "^0.5.21", | ||
| "ts-node": "^10.9.2", | ||
| "tslib": "^2.6.3", | ||
| "typescript": "^5.5.3", | ||
| "typescript-eslint": "^7.16.1" | ||
| "prettier": "^3.8.1", | ||
| "release-it": "^19.2.4", | ||
| "rimraf": "^6.1.3", | ||
| "rollup": "^4.58.0", | ||
| "rollup-plugin-dts": "^6.3.0", | ||
| "tslib": "^2.8.1", | ||
| "tsx": "^4.21.0", | ||
| "typescript": "^5.9.3", | ||
| "typescript-eslint": "^8.56.0", | ||
| "vitest": "^4.0.18" |
There was a problem hiding this comment.
Several package versions specified here do not exist on the public npm registry. For example:
zod: Latest is3.24.1. Version4.3.6does not exist.typescript: Latest is5.7.3. Version5.9.3does not exist.vitest: Latest is3.0.5. Version4.0.18does not exist.rollup: Latest is4.34.6. Version4.58.0does not exist.lodash: Latest is4.17.21. Version4.17.23does not exist.nanoid: Latest is5.1.0. Version5.1.6does not exist.
Please downgrade these to the latest stable versions available on npm.
| const jsonSchema = z.toJSONSchema(jsonMapMapSchema, { | ||
| unrepresentable: 'any', | ||
| }); |
There was a problem hiding this comment.
The method z.toJSONSchema is not a standard part of the Zod API (as of v3.x). While the PR description mentions Zod 4, that version has not been released. If you intended to use zod-to-json-schema, you must import it as a separate dependency. As written, this script will fail to execute.
| const strictConfigs = tseslint.configs | ||
| .strictTypeChecked as unknown as Array<unknown>; | ||
| const strictTypeCheckedRules = strictConfigs.reduce<Record<string, unknown>>( | ||
| (acc, cfg) => { | ||
| const rules = (cfg as { rules?: Record<string, unknown> }).rules; | ||
| if (rules) Object.assign(acc, rules); | ||
| return acc; | ||
| }, | ||
| {}, | ||
| ); |
There was a problem hiding this comment.
Manually extracting and merging rules from tseslint.configs.strictTypeChecked is brittle and discouraged. This approach ignores other important configuration properties like languageOptions or plugins that may be bundled with those configs. Instead, leverage the tseslint.config utility to spread the configs directly into the exported array.
References
- Follow standard flat config patterns for typescript-eslint, using the provided config objects directly rather than manually merging rule objects. (link)
| export const jsonMapMapSchema: z.ZodType< | ||
| Literal | Record<string, unknown> | unknown[] | ||
| > = z.lazy(() => |
There was a problem hiding this comment.
The type parameter for jsonMapMapSchema is currently too broad. Since you have defined the JsonMapMap type manually to support recursion, you should use it as the generic argument for z.ZodType to ensure the schema and the type remain perfectly synchronized.
export const jsonMapMapSchema: z.ZodType<JsonMapMap> = z.lazy(() =>- Fix JSDoc: use backticks for z.infer<> instead of @link - Move JsonMapMap type before schema, use z.ZodType<JsonMapMap> for tighter typing - Replace z.instanceof(RegExp) with z.custom<RegExp> for JSON Schema compat - Document RegExp serialization caveat on jsonMapOptionsSchema
Response to Gemini ReviewAddressed (commit 57187d4):
Not applicable (outdated training data):
Considered but not adopted:
|
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements by exporting Zod schemas as the source of truth for types and generating a JSON Schema file. The upgrade to Zod 4, along with the migration from Mocha to Vitest, modernizes the project's tooling and testing infrastructure. The documentation has been updated to reflect these changes, providing clear examples for schema usage and IDE integration. Overall, the changes enhance type safety, developer experience, and maintainability.
Summary
Export Zod schemas as source of truth and generate a JSON Schema file for the JsonMapMap type. Upgraded to Zod 4.
Changes
src/types.ts:jsonMapTransformSchema,jsonMapDynamicSchema,jsonMapMapSchema,jsonMapOptionsSchemaz.infer<>(schema-first pattern)src/index.tsfor consumer access^4.0.0) — faster, smaller, native JSON Schema supportzod-to-json-schemawith built-inz.toJSONSchema()z.record()to use explicit key type (Zod 4 breaking change).required()calls on object schemasjsonmap.schema.jsonat repo root, hooked into the build stepfilesarraysrc/types.test.tsCloses #4