Improving final intersection and fixing it for loose object schema #2762
Conversation
RobinTail
added
enhancement
WalkthroughThe changes update core type definitions to allow input schemas to be Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant EndpointsFactory
participant Handler
Developer->>EndpointsFactory: build({ input: schema | undefined, ... })
EndpointsFactory->>Handler: handler({ input: ConditionalIntersection<MIN, IN> })
Handler-->>EndpointsFactory: Returns output
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
express-zod-api/src/io-schema.ts (1)
10-14: LGTM: Well-designed conditional intersection typeThe
AltIntersectiontype elegantly handles the case where middleware input schemas might be undefined. The conditional logic correctly:
- Uses
Currentwhen it's a validIOSchema- Falls back to
IncwhenCurrentis undefinedThis enables flexible schema composition while maintaining type safety.
Note: The TODO comment suggests naming uncertainty - consider more descriptive names like
ConditionalIntersectionorOptionalIntersectionif this becomes a public API.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
express-zod-api/src/endpoints-factory.ts(5 hunks)express-zod-api/src/io-schema.ts(2 hunks)express-zod-api/tests/endpoints-factory.spec.ts(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
express-zod-api/tests/endpoints-factory.spec.ts (1)
express-zod-api/src/endpoints-factory.ts (1)
defaultEndpointsFactory(168-170)
express-zod-api/src/io-schema.ts (2)
express-zod-api/src/index.ts (1)
IOSchema(49-49)express-zod-api/src/middleware.ts (1)
schema(82-84)
express-zod-api/src/endpoints-factory.ts (3)
express-zod-api/src/io-schema.ts (2)
IOSchema(8-8)AltIntersection(11-14)express-zod-api/src/endpoint.ts (2)
Handler(36-43)output(201-207)express-zod-api/src/common-helpers.ts (2)
FlatObject(13-13)EmptyObject(10-10)
🔇 Additional comments (12)
express-zod-api/tests/endpoints-factory.spec.ts (4)
3-3: LGTM: Import addition for stricter type testingThe addition of
expectTypeOfimport supports the enhanced type testing approach used in the updated test cases.
86-106: LGTM: Comprehensive test coverage for Issue #2760These new tests properly validate the input property handling behavior:
- First test ensures excessive properties are stripped by default (line 91 confirms
inputdoesn't havebarproperty)- Second test verifies loose object schemas allow excessive properties as
unknowntype (line 102 confirmsbarproperty exists with correct type)The tests directly address the stated Issue #2760 and provide good coverage for the new intersection type handling.
118-118: LGTM: Consistent with new undefined input schema supportThe change from
EmptySchematoundefinedaligns with the updated generic parameter defaults inEndpointsFactorywhereINnow defaults toundefinedinstead ofEmptySchema.
275-277: LGTM: Enhanced type safety with stricter equality checksThe change from
toExtendtotoEqualTypeOfprovides stricter type checking, ensuring the inferred types exactly match the expected intersection types rather than just extending them. This improvement aligns with the enhanced type handling introduced in theAltIntersectionimplementation.Also applies to: 302-304, 325-327, 351-352
express-zod-api/src/io-schema.ts (2)
24-24: LGTM: Consistent generic parameter updateThe change from
IOSchematoIOSchema | undefinedfor theMINparameter aligns with the newAltIntersectiontype requirements and supports cases where no middleware input schema is defined.
32-32: LGTM: Proper usage of new intersection typeThe return type correctly uses
AltIntersection<MIN, IN>instead of the directz.ZodIntersection<MIN, IN>, which enables proper type handling whenMINmight be undefined. The runtime behavior remains unchanged, maintaining backward compatibility.express-zod-api/src/endpoints-factory.ts (6)
5-9: LGTM: Consistent import updateThe addition of
AltIntersectionto the imports properly supports the updated type usage throughout the file.
25-25: LGTM: Interface parameter update aligns with new type systemThe change from
IOSchematoIOSchema | undefinedfor theMINparameter inBuildPropsis consistent with the overall support for optional middleware input schemas.
38-38: LGTM: Handler type properly uses new intersection typeThe handler type correctly uses
AltIntersection<MIN, IN>instead of the directz.ZodIntersection<MIN, IN>, ensuring proper type inference when middleware input schemas might be undefined.
66-66: LGTM: Consistent generic parameter defaultChanging the
INgeneric default fromEmptySchematoundefinedaligns with the new type system and is consistent with the test changes that expectundefinedinstead ofEmptySchema.
74-74: LGTM: Private method parameter update is consistentThe change to allow
undefinedfor theCINgeneric parameter in the#createmethod maintains consistency with the overall type system updates.
93-93: LGTM: Method return type properly uses new intersection typeThe
addMiddlewaremethod correctly usesAltIntersection<IN, AIN>in the returned factory's generic parameters, ensuring proper type composition when combining input schemas.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (2)
7-11: Wording can be tightened for clarityThe second and third bullet points read a bit awkwardly (“This also fixes the issue for excessive properties…”).
Consider re-phrasing to make the subject clearer and keep the tense consistent with the first bullet:- - This also fixes the issue for excessive properties of the `z.looseObject()` based schema; + - Fixed incorrect typing of excessive properties when using a `z.looseObject()`-based schema;Likewise, “The issue reported by” → “Issue reported by” removes an extraneous article.
12-33: Fence indentation breaks Markdown highlightingBecause the outer block is already inside a fenced code block (started with ````), the inner TypeScript fence is indented by two spaces.
GitHub’s markdown renderer treats that as part of the code block rather than a nested fence, so the syntax highlighting is lost.Unindent the inner triple back-ticks by two spaces:
-```ts +```ts import { defaultEndpointsFactory } from "express-zod-api"; import { z } from "zod/v4"; @@ });(Only the back-ticks need to be moved flush-left; the code sample can stay indented.) </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between f38c8596461d197a192e5d0c4ee48f7ce8c3f17b and 1901050f1ad0474065b646b90f86a22ccc5782c9. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `CHANGELOG.md` (1 hunks) * `README.md` (1 hunks) </details> <details> <summary>✅ Files skipped from review due to trivial changes (1)</summary> * README.md </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms (7)</summary> * GitHub Check: build (24.0.0) * GitHub Check: build (24.x) * GitHub Check: build (20.9.0) * GitHub Check: build (22.x) * GitHub Check: build (22.0.0) * GitHub Check: build (20.x) * GitHub Check: Analyze (javascript) </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
RobinTail
left a comment
RobinTail
left a comment
There was a problem hiding this comment.
well, ok.
I should invent something better in v25
RobinTail
changed the title
…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.
Fixes #2760
Alternative implementation to #2761
I like this one more for being more explicit.
Summary by CodeRabbit
New Features
Bug Fixes
nevertypes.Tests
Documentation