Skip to content

Conversation

@RobinTail
Copy link
Owner

@RobinTail RobinTail commented Jan 11, 2026

Related to #3166

This fix prevents ctx from having unions with Record<string, never>, but it does not change the type of ctx in defaultEndpointsFactory that remains Record<string, never>.

The scope of the change are only the factories having at least one Middleware attached (that makes a union for ctx)

Summary by CodeRabbit

  • Refactor

    • Improved type inference for endpoints when no context is provided, simplifying type composition to avoid empty-object intersections.
  • Tests

    • Updated type expectations to reflect the simplified type shape.
  • Documentation

    • Added a changelog entry describing the type fix.
    • Added a contributor entry to the README.
  • Chores

    • Bumped package version to a pre-release.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 11, 2026

📝 Walkthrough

Walkthrough

The PR refines EndpointsFactory's internal #extend generic: the CTX parameter now collapses to RET when CTX is EmptyObject via (CTX extends EmptyObject ? RET : CTX) & RET, updating type inference for empty contexts and adjusting corresponding tests and changelog/README entries.

Changes

Cohort / File(s) Summary
Type parameter refinement
express-zod-api/src/endpoints-factory.ts
Changed #extend instantiation second generic from CTX & RET to (CTX extends EmptyObject ? RET : CTX) & RET, preventing EmptyObject from leaking into inferred ctx types.
Test type expectations
express-zod-api/tests/endpoints-factory.spec.ts
Updated test type assertions: replaced EmptyObject & { ... } with { ... } in EndpointsFactory<..., ...> expectations to match new inference.
Changelog & contributors
CHANGELOG.md, README.md
Added v26.3.1 changelog entry describing the ctx type fix and credited contributor pycanis; README updated contributors list.
Package metadata
express-zod-api/package.json
Bumped version to 26.3.1-beta.1.

Sequence Diagram(s)

(Skipped — changes are type-level/type-inference adjustments without new runtime control flow.)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

documentation

Poem

🐰 I nibble types with careful paws,
Empty crumbs tucked out of laws,
Contexts snug where they belong,
Middleware hums a clearer song,
Hooray — a hop, a tidy cause! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix: Avoid union with EmptyObject for ctx' accurately describes the main technical change—preventing union types with EmptyObject for the ctx parameter in factories with middleware.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d8f8c27 and 1ebf969.

📒 Files selected for processing (2)
  • express-zod-api/package.json
  • express-zod-api/src/endpoints-factory.ts
✅ Files skipped from review due to trivial changes (1)
  • express-zod-api/package.json
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-28T18:58:10.064Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on types/qs. This import provides the reference TypeScript needs to infer portable type names.

Applied to files:

  • express-zod-api/src/endpoints-factory.ts
📚 Learning: 2025-08-08T11:59:04.814Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 2878
File: zod-plugin/runtime.ts:39-42
Timestamp: 2025-08-08T11:59:04.814Z
Learning: express-zod-api/zod-plugin/runtime.ts (TypeScript): Do not memoize the ZodType.prototype.brand getter. The current design returning setBrand.bind(this) on each access is intentional/preferred; avoid redefining "brand" as a data property per instance.

Applied to files:

  • express-zod-api/src/endpoints-factory.ts
🧬 Code graph analysis (1)
express-zod-api/src/endpoints-factory.ts (1)
express-zod-api/src/common-helpers.ts (1)
  • EmptyObject (19-19)
🔇 Additional comments (1)
express-zod-api/src/endpoints-factory.ts (1)

82-95: LGTM! Clean type-level improvement.

The conditional type logic correctly collapses EmptyObject & RET to just RET, eliminating redundant intersections in the type signature. When CTX is not empty, the behavior remains identical (CTX & RET). This achieves the stated goal of preventing ctx from having unions with Record<string, never> while preserving type safety.

The implementation is sound:

  • When starting with an empty context: (RET) & RET simplifies to RET
  • When context is already populated: (CTX) & RET maintains the original behavior

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.

@coveralls-official
Copy link

coveralls-official bot commented Jan 11, 2026

Coverage Status

coverage: 100.0%. remained the same
when pulling 7775172 on fix-ctx-empty-object-union
into 2749eca on master.

@RobinTail RobinTail marked this pull request as ready for review January 11, 2026 09:06
@RobinTail RobinTail merged commit 0460d95 into master Jan 12, 2026
5 checks passed
@RobinTail RobinTail deleted the fix-ctx-empty-object-union branch January 12, 2026 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants