-
-
Notifications
You must be signed in to change notification settings - Fork 35
v25 is for Sara Millerey González
#2741
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
Conversation
WalkthroughThis update transitions the project to a pure ESM (ECMAScript Module) distribution, removes all references to Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EndpointFactory
participant Middleware
participant Endpoint
participant Zod
participant globalRegistry
User->>EndpointFactory: addMiddleware(mw)
EndpointFactory->>Middleware: Access schema (may be undefined)
EndpointFactory->>EndpointFactory: ensureExtension(schema, mw.schema)
EndpointFactory->>Endpoint: build(inputSchema = makeFinalInputSchema(factorySchema, buildSchema))
Endpoint->>Zod: Validate input/output schemas
Endpoint->>globalRegistry: Access examples metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…#2740) Fixed in zod 3.25.61: colinhacks/zod@1c2ad87#comments Elevating peer dependency requirement is breaking, therefore addressing to v25
After elevating zod version those two no longer needed anymore
This should be default when `@types/node` get updated to v24. Related to #2587 Related to #2619 The breaking change is that it requires Node 20.18.1, while it works fine with 20.9.0. The dependency, however, is only used for testing, so it's still a questionable move. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Updated Node.js engine requirements to a newer minimum version in the 20.x range. - Upgraded the "undici" development dependency to the latest version. - Adjusted CI workflow to test against the updated Node.js version. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This version requires higher Node 20 and 22 The Node 20 part seems to be aligned with #2622
…default (#2777) Related to #2762 This should simplify intersections, remove redundant EmptySchema when it's not really needed, keeping it only for the final one. I also realized that doing final intersection within `::build()` makes it intersect middleware schemas for every endpoint built on the factory. It can be simpler now.
# Conflicts: # pnpm-lock.yaml # pnpm-workspace.yaml
Previously, I separated core types imports because they were intended to be published in a separate package `@zod/core`. However, since stable v4 rolled out, that idea was abandoned, so that it's now safe to refer to it using the `z.core` without additional imports. This was suggested by @coderabbitai <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Updated internal type references to use the public `z.core` namespace instead of direct imports from internal Zod paths. This affects type annotations and function signatures throughout the codebase, including helpers, schemas, documentation utilities, and tests. * **Tests** * Adjusted test files to use the new `z.core` type references, ensuring consistency with the updated type import approach. No changes to application logic or user-facing features. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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 pull request implements the major version 25 of the express-zod-api framework, which includes several breaking changes focused on modernizing the codebase and improving developer experience. The changes center around upgrading to Zod v4, transitioning to ESM-only distribution, and simplifying the API.
Key changes include:
- Zod v4 Migration: Updates all imports from
zod/v4tozodand removes compatibility with Zod v3 - ESM-Only Distribution: Removes CommonJS support and updates build configuration for pure ESM output
- API Simplification: Removes deprecated features like the
getExamples()helper and legacy example metadata formats
Reviewed Changes
Copilot reviewed 98 out of 99 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tsup.base.ts | Updates build configuration to output ESM format only |
| pnpm-workspace.yaml | Removes deprecated dependencies and version overrides |
| migration/* | Updates migration tooling for v25 with new ESLint rules and version bumps |
| express-zod-api/src/* | Updates all Zod imports from zod/v4 to zod and removes legacy example handling |
| express-zod-api/tests/* | Updates test imports and removes deprecated functionality tests |
| example/* | Updates example code to use new Zod import syntax |
| package.json files | Updates version numbers and removes CommonJS export configurations |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
RobinTail
left a comment
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.
reviewed.
all good. don't forget to create v24 branch before merging
# Conflicts: # express-zod-api/package.json # express-zod-api/src/documentation-helpers.ts # pnpm-lock.yaml
RobinTail
left a comment
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.
ready
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: 1
🔭 Outside diff range comments (1)
express-zod-api/tests/system.spec.ts (1)
1-6:assertis used but no longer imported – tests will crash at runtimeThe file calls
assert(...)multiple times (e.g., Line 43, 150 …), yet theassertmodule import was accidentally dropped when the import block was modified. This will raiseReferenceError: assert is not definedduring test execution.import cors from "cors"; import depd from "depd"; import express from "express"; import { readFile } from "node:fs/promises"; +import assert from "node:assert/strict"; import { z } from "zod";
♻️ Duplicate comments (2)
express-zod-api/src/startup-logo.ts (1)
15-15: LGTM! Dedication message updated appropriately.The dedication message has been updated from "for Ashley" to "for Sara" as part of version 25 dedication to Sara Millerey González, as mentioned in the PR objectives. The alignment has already been verified.
migration/index.ts (1)
21-21: Migration rule missing support for "zod/v4/core" importsThe current query only matches
"zod/v4"imports but misses"zod/v4/core"imports. According to the PR comments, there are 17 files still importing from"zod/v4/core"which will break when/v4compatibility is dropped.Update the zod query to handle both patterns:
- zod: `${NT.ImportDeclaration}[source.value='zod/v4']`, + zod: `${NT.ImportDeclaration}[source.value=/^zod\/v4/]`,And update the fix to handle the core subpath:
- fix: (fixer) => fixer.replaceText(node.source, `"zod"`), + fix: (fixer) => { + const currentValue = node.source.value as string; + const newValue = currentValue.replace(/^zod\/v4/, "zod"); + return fixer.replaceText(node.source, `"${newValue}"`); + },Also applies to: 56-62
🧹 Nitpick comments (5)
express-zod-api/tests/index.spec.ts (1)
3-3: Relying onz.core.*internal types may be brittleSwitching from explicit
"zod/v4/core"imports toz.core.$ZodType/z.core.JSONSchemakeeps the tests compiling, but thecorenamespace is still undocumented internal API.
If upstream renames it, the public contract of your library’s types still compiles but the tests will break. Consider pinning types with a// @ts-expect-error upstream internalcomment or re-exporting the needed aliases in a local helper to isolate the surface area.Also applies to: 44-45
express-zod-api/tests/deep-checks.spec.ts (1)
2-2: Samez.coredependency caveat applies hereThe test now uses
z.core.$ZodTypeandz.core.$brand. See earlier note: these remain internal to Zod. Isolating them behind your own re-exports will future-proof the suite.Also applies to: 10-10, 26-26
CHANGELOG.md (3)
7-9: Drop the trailing colon after the bullet headerLine 7 ends with a colon, which makes the bullet look unfinished:
- Supported Node.js versions: `^20.19.0 || ^22.12.0 || ^24.0.0`:Unless you intend to introduce a sub-list right after it, remove the trailing colon for consistency with the surrounding bullets.
15-16: Avoid MUST/SHOULD in user-facing changelog proseUsing RFC-style capitals (“SHOULD”) feels out of place in a changelog aimed at developers and breaks the otherwise sentence-like tone. Recommend replacing with lower-case “should” (or rephrasing).
25-31: Minor markdown nit – the fenced diff loses syntax highlightingThe long triple-back-tick block immediately followed by another triple-back-tick opens a diff block inside a diff fence, which disables syntax colouring in many renderers. Prefer using separate fenced blocks:
- z.string().meta({ example: "test" }); ...then close and start a new ```diff for the second snippet.
This keeps GitHub’s renderer happy.
@coderabbitai, why do you think so? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@RobinTail You're absolutely right to question this - let me examine the actual current state to understand what approach was taken and clarify my assessment. 🏁 Script executed: Length of output: 3024 🏁 Script executed: Length of output: 2047 🏁 Script executed: Length of output: 2816 @RobinTail You are absolutely correct, and I apologize for the confusion in my assessment. Looking at the actual implementation, I can see that: ✅ The Migration Was Completed SuccessfullyWhat Actually Happened:
Current State (Working Correctly):
🤦♂️ My ErrorYou're right to point out the contradiction in my feedback. I was wrong when I said:
✅ The Correct UnderstandingThe migration strategy you implemented is exactly right:
This is a clean, working solution that properly drops the Thank you for the correction - the implementation is solid and my previous concerns were unfounded. ✏️ Learnings added
🧠 Learnings used |
RobinTail
left a comment
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.
QA passed
Dedication
Sara Millerey González was a 32 years young trans woman from Bello, Antioquia, Colombia.
She was killed in April 2025. Millerey was raped, tortured, and thrown into a ravine by a group of men. Denounced as a hate crime by press and Antioquia's mayor, the Colombian National Working Group on Violence Based on Victims' Sexual Orientation or Gender Identity is collaborating with the Attorney's General's Office on identifying remaining suspects.
Transgender women suffer too frequently from transphobic violence and cruelty, being the less protected social group. I'd like to raise an awareness of this problem. Humans should be creators — not killers. But most importantly, I want every transgender girl to have an opportunity to create applications quickly and, in general, learn to write code easily in order to receive job offers and leave dangerously transphobic territories for more favorable and civilized ones, and live happily there. Protect transgender women.
Breaking
zodversion is^4.0.0;zod@^3is dropped;import { z } from "zod"without the/v4suffix;^20.19.0 || ^22.12.0 || ^24.0.0;require(ESM)syntax;Middlewareclass:inputschema is not defined, theinputargument of thehandlermethod is nowunknown;exampleremoved;examplesremoved (only array remains);.example()or.meta({})withexamplesbeing an array.getExamples()helper removed:.meta()?.examplesorglobalRegistry.get()?.examplesinstead.Content
undici7 #2622z.object({})for describingEmptySchemaandEmptyObject#2740undefinedinput schema for factories and middlewares by default #2777import from "zod"#2809vitest4 #2815example, object-basedexamplesandgetExamples()#2842z.core#2844Summary by CodeRabbit
Summary by CodeRabbit
New Features
"zod"directly.Breaking Changes
getExamples()helper; access examples via.meta()?.examplesor the global registry.undefinedwhen omitted, not an empty object schema.examplesin schema metadata; use.example()or array-basedexamplesinstead."zod/v4"to"zod"throughout codebase and examples.Bug Fixes
Chores
undiciand adjusted package metadata.Tests
Style
Revert