Skip to content

fix(toJSONSchema): RFC 6901 encode ref pointer tokens when id contains / or ~#6084

Open
1795771535y-cell wants to merge 6 commits into
colinhacks:mainfrom
1795771535y-cell:fix/json-schema-ref-encoding-v2
Open

fix(toJSONSchema): RFC 6901 encode ref pointer tokens when id contains / or ~#6084
1795771535y-cell wants to merge 6 commits into
colinhacks:mainfrom
1795771535y-cell:fix/json-schema-ref-encoding-v2

Conversation

@1795771535y-cell

Copy link
Copy Markdown

Closes #6027

Per RFC 6901, JSON Pointer reference tokens must escape ~ as ~0 and / as ~1.

Previously, toJSONSchema() used the raw schema id (from .meta()) directly in ref paths without encoding these characters. This broke JSON Schema consumption when the id contained / or ~.

Example:

const User = z.object({ name: z.string() }).meta({ id: 'Shared/User~' });
z.toJSONSchema(z.object({ User }));

Before: ref: #/defs/Shared/User~ (invalid pointer)
After: ref: #/defs/Shared~1User~0 (valid RFC 6901 pointer)

The defs key remains as the original unencoded id.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one test-coverage suggestion.

Reviewed changes — RFC 6901-encodes JSON Pointer reference tokens in toJSONSchema when the schema's .meta({ id }) contains ~ or /, fixing #6027.

  • Add encodeRefToken helper — encodes ~~0 and /~1 for JSON Pointer reference tokens in extractDefs.
  • Apply encoding in both ref paths — the self-contained #/$defs/... ref path and the external #/__shared/... registry ref path are both updated. The $defs object key remains the original unencoded id.

ℹ️ Missing test coverage for the fix

This is a bug fix for an observable behavior regression (invalid $ref pointers when id contains / or ~), but no test is added. No existing test exercises an id with these characters, so the fix has zero coverage.

Technical details
# Missing test coverage for the fix

## Affected sites
- `packages/zod/src/v4/core/to-json-schema.ts:266``__shared` path uses `encodeRefToken(id)`
- `packages/zod/src/v4/core/to-json-schema.ts:277` — self-contained ref path uses `encodeRefToken(defId)`

## Required outcome
- Add at least one test that verifies `$ref` tokens are encoded when the schema `id` contains `/` and/or `~`, for both the self-contained and external-registry paths. The inline snapshot tests in `to-json-schema.test.ts` are a good fit.

## Suggested approach
A lightweight inline snapshot test covering both characters together:

```ts
test("ref tokens are RFC 6901 encoded when id contains / or ~", () => {
  const inner = z.string().meta({ id: "Shared/User~" });
  const result = z.toJSONSchema(z.object({ a: inner }));
  expect(result.$defs!["Shared/User~"]).toEqual({ type: "string" });
  expect((result.properties!.a as any).$ref).toBe("#/$defs/Shared~1User~0");
});
```

Pullfrog  | Fix it ➔View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@1795771535y-cell

Copy link
Copy Markdown
Author

@pullfrog Thank you for the suggestion! I've added a test case in \packages/zod/src/v4/classic/tests/to-json-schema.test.ts\ that verifies RFC 6901 encoding works correctly when the schema id contains /\ and ~\ characters.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — added the test-coverage suggested in the prior review, plus an inline comment noting the RFC 6901 encoding.

  • Added test for RFC 6901 encodingto-json-schema.test.ts now exercises the $ref token encoding when .meta({ id }) contains / and ~, verifying both the $defs key and the encoded $ref path.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

The formatter complained about an extra blank line between test cases.
This fix removes the extra blank line to satisfy the linter.

Fixes CI failure in colinhacks#6084

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The latest commit (ff6bf627) introduced a UTF-8 BOM at the start of to-json-schema.test.ts and corrupted an emdash in an existing comment. Both need to be reverted.

Reviewed changes — the latest commit attempted to remove an extra blank line in the test file, but introduced file corruption in the process.

  • BOM character addedto-json-schema.test.ts now starts with a UTF-8 BOM (EF BB BF), which can cause issues with toolchains, linters, and source-map processors.
  • Emdash corrupted — the on line 2177 was replaced with the mojibake sequence 鈥?.
  • Core fix unchanged — the encodeRefToken implementation in to-json-schema.ts is correct and unchanged from the prior review.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

Comment thread packages/zod/src/v4/classic/tests/to-json-schema.test.ts Outdated

test("id is stripped from $defs entries (draft-2020-12)", () => {
// The `id` in `.meta()` is a registration tag it determines the $defs key
// The `id` in `.meta()` is a registration tag 鈥?it determines the $defs key

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The emdash (U+2014) was corrupted to the mojibake sequence 鈥? (U+9265 U+003F). Revert to the original emdash character that was present in commit 9598525a.

The formatter complained about an extra blank line between test cases.
This fix removes the extra blank line to satisfy the linter.

Fixes CI failure in colinhacks#6084

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

The latest commit (72e8e988) deleted the entire to-json-schema.test.ts file — all 3140 lines including the RFC 6901 encoding test that was added in this PR.

Reviewed changes — the latest commit attempted to fix the BOM/emdash corruption flagged in the prior review but instead nuked the file.

  • Deleted entire test file72e8e988 removed all 3140 lines of to-json-schema.test.ts, leaving it empty (0 lines, 0 bytes). The to-json-schema.ts core fix is correct and unchanged.

🚨 to-json-schema.test.ts has been deleted

Commit 72e8e988 (fix: remove extra blank line in test file) emptied the test file instead of stripping the BOM and restoring the corrupted emdash. The working tree now has zero content in that file — all existing toJSONSchema test coverage and the newly added RFC 6901 encoding test are gone.

Technical details
# to-json-schema.test.ts has been deleted

## Affected sites
- `packages/zod/src/v4/classic/tests/to-json-schema.test.ts` — 0 lines, 0 bytes (was 3140 lines). The empty blob is `e69de29b`.

## Required outcome
- Restore `to-json-schema.test.ts` to its state at commit `9598525a` (the last working version with the correct RFC 6901 test and no BOM/corruption). That is: `git checkout 9598525a -- packages/zod/src/v4/classic/tests/to-json-schema.test.ts` then commit that as the fix.

## Suggested approach
Checkout the file from the last known-good state and commit it cleanly — no need to manually reconstruct anything.

```bash
git checkout 9598525a -- packages/zod/src/v4/classic/tests/to-json-schema.test.ts
git commit -m "fix: restore test file deleted by previous commit"
```

Pullfrog  | Fix it ➔View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The test-file restore brought back the base-branch content, which drops the RFC 6901 encoding test that was added earlier in this PR.

Reviewed changes — commit 2ffa0dbd restored to-json-schema.test.ts after it was deleted in the prior commit.

  • Restored to-json-schema.test.ts — the file is back as a proper 3131-line UTF-8 file with no BOM. The encodeRefToken core fix in to-json-schema.ts is correct and unchanged.

⚠️ RFC 6901 encoding test is missing

The file was restored from the base branch rather than from commit 9598525a, so the test that was explicitly added for this fix is gone. The encodeRefToken helper has no test coverage.

Technical details
# RFC 6901 encoding test is missing

## Affected sites
- `packages/zod/src/v4/classic/tests/to-json-schema.test.ts` — the test added in `9598525a` (which exercised `encodeRefToken` with `Shared/User~``Shared~1User~0`) is absent.

## Required outcome
- Add the RFC 6901 encoding test back. The test from `9598525a` is:

```ts
test("ref tokens are RFC 6901 encoded when id contains / or ~", () => {
  const inner = z.string().meta({ id: "Shared/User~" });
  const result = z.toJSONSchema(z.object({ a: inner }));
  expect(result.$defs!["Shared/User~"]).toEqual({ type: "string" });
  expect((result.properties!.a as any).$ref).toBe("#/$defs/Shared~1User~0");
});
```

## Suggested approach
Add the test block after the other `toJSONSchema` tests — the core fix is already in place; only the test is missing.

Pullfrog  | Fix it ➔View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: toJSONSchema $ref pointer not encoded when meta id contains / or ~

1 participant