Skip to content

Conversation

HarryWang29
Copy link

@HarryWang29 HarryWang29 commented Oct 23, 2025

use the same way as zod3

@vercel
Copy link

vercel bot commented Oct 23, 2025

@HarryWang29 is attempting to deploy a commit to the colinhacks Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

Walkthrough

The iteration method in the handleCatchall function within the schemas file was switched from Object.keys() to a for...in loop. This change expands which properties get iterated over—Object.keys() only covers own enumerable properties, while for...in also includes inherited enumerable properties from the prototype chain. This shift could affect which keys are classified as unrecognized or passed to the catchall handler, though no other logic modifications were made.

Pre-merge checks

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ❓ Inconclusive No pull request description was provided by the author, so there's nothing concrete to evaluate against the changeset. While the check is lenient and doesn't require extensive documentation, having nothing to assess against makes it impossible to determine whether a description would be related to the changes or not. Consider adding a brief description explaining why this iteration method change is necessary—whether it's to include inherited properties, fix a bug, or improve behavior. Even a couple sentences about the intent would help future maintainers understand the reasoning behind the switch.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Fix: Iterate over keys in catchall object using 'in' operator" directly and clearly describes the main change made in the changeset. The modification replaces Object.keys() iteration with the in operator, which is exactly what the title conveys. The title is specific, concise, and gives reviewers a clear understanding of what this PR is about without needing to dig into the code.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d336c4 and c4d2314.

📒 Files selected for processing (1)
  • packages/zod/src/v4/core/schemas.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx,mjs,cjs,json}

📄 CodeRabbit inference engine (CLAUDE.md)

Enforce line width of 120 characters via Biome formatting

Files:

  • packages/zod/src/v4/core/schemas.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}

📄 CodeRabbit inference engine (CLAUDE.md)

Use ES5-style trailing commas in JavaScript/TypeScript code

Files:

  • packages/zod/src/v4/core/schemas.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Allow the any type in TypeScript (noExplicitAny off)
Allow non-null assertions in TypeScript (noNonNullAssertion off)
Write TypeScript to pass strict mode with exactOptionalPropertyTypes enabled
Use NodeNext module resolution semantics for imports in TypeScript
Target ES2020 language features in TypeScript source

Files:

  • packages/zod/src/v4/core/schemas.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}

📄 CodeRabbit inference engine (CLAUDE.md)

Allow parameter reassignment for performance-sensitive code (noParameterAssign off)

Files:

  • packages/zod/src/v4/core/schemas.ts
**/*.{js,mjs,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)

**/*.{js,mjs,ts,tsx}: Use .js extensions in import specifiers (e.g., import { z } from "./index.js")
Don’t use require(); use ESM import statements

Files:

  • packages/zod/src/v4/core/schemas.ts
**/*.{js,mjs,cjs,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)

Do not leave log statements (e.g., console.log, debugger) in tests or production code

Files:

  • packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/core/{schemas.ts,core.ts}

📄 CodeRabbit inference engine (.cursor/rules/zod-internals.mdc)

Use the custom constructor system via core.$constructor() and initialize instances with $ZodType.init() when creating schemas

Files:

  • packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/core/schemas.ts

📄 CodeRabbit inference engine (.cursor/rules/zod-internals.mdc)

packages/zod/src/v4/core/schemas.ts: Wrapper schemas (e.g., $ZodOptional, $ZodNullable, $ZodReadonly) must pass through internal properties from their inner type using util.defineLazy for propValues, values, optin, and optout
Define computed internal properties using util.defineLazy() to avoid circular dependencies
Implement schema parse functions following the standard structure: type check, push invalid_type issue on mismatch, optionally coerce/transform, and return payload
For $ZodDiscriminatedUnion, compute and merge propValues lazily from options; ensure each option provides the discriminator key and that values are unique
Ensure readonly wrapper types (e.g., $ZodReadonly) pass through values for discriminator support in unions

Files:

  • packages/zod/src/v4/core/schemas.ts
packages/zod/src/v4/core/{schemas.ts,checks.ts}

📄 CodeRabbit inference engine (.cursor/rules/zod-internals.mdc)

When adding issues, push well-formed payload.issues entries including code, expected (when applicable), input, inst, and optional path/message/continue

Files:

  • packages/zod/src/v4/core/schemas.ts
packages/**/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/zod-project-guide.mdc)

Write source code in TypeScript (TypeScript-first codebase)

Files:

  • packages/zod/src/v4/core/schemas.ts
packages/zod/**

📄 CodeRabbit inference engine (.cursor/rules/zod-project-guide.mdc)

Make core Zod library changes in the main package at packages/zod/

Files:

  • packages/zod/src/v4/core/schemas.ts
🧬 Code graph analysis (1)
packages/zod/src/v4/core/schemas.ts (1)
packages/zod/src/v4/core/core.ts (1)
  • input (107-107)
🔇 Additional comments (1)
packages/zod/src/v4/core/schemas.ts (1)

1786-1786: Clarify intent: This change from Object.keys() to for...in was intentional per commit c4d2314, but needs verification.

The switch to for (const key in input) is confirmed as a deliberate fix (commit Oct 23). However, the technical concern is valid: for...in iterates inherited properties while Object.keys() only returns own properties. This behavioral difference hasn't been explicitly documented or tested.

Before merging, please confirm:

  1. Why was the change from Object.keys() necessary? What bug does for...in fix?
  2. Is iterating inherited properties the desired behavior? Should prototype-chain properties be validated by catchall?
  3. Add a guard or document the change – Either add if (!Object.hasOwnProperty.call(input, key)) continue; if inherited properties shouldn't be processed, or add a comment + test explaining why they should be.

Note: All three for...in loops (lines 1786, 1840, 2610) lack this guard, so the intent affects multiple code paths.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

1 participant