fix(effect): default middlewareGen type parameters so standalone middleware works on concrete procedures - #1996
Conversation
…leware works on concrete procedures
5eb68c0 to
9ab2359
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Default
middlewareGentype parameters —packages/effect/src/middleware.tsgivesTInput = unknown,TOutput = any,TErrorMap = Record<never, never>, andTYielddefaults, and movesTOutContextto the second generic position, mirroring core'sBuilder.middleware($Output = any) so a standalone generator middleware can be.used on procedures with concrete input/output. - Type test for standalone middleware on concrete procedures —
middleware.test-d.tsattaches a standalone generator middleware to a concrete-input/output procedure via both.use()andos.middleware(...), asserting context flows through.
I verified pnpm --filter @orpc/experimental-effect type:check passes with the new test, and eslint is clean on both files. The test is genuine coverage: under the old signature middlewareGen<ServerContext, { user: 'user' }> bound the second arg to TInput and left TOutput/TErrorMap unsatisfied (plus TOutput = unknown would break .use regardless), so it fails on the old code.
One thing worth keeping in mind (already acknowledged in the PR description): the parameter reorder is not a compiler error for external callers who passed explicit type args in the old order — those get silently remapped positionally. Given only the previously-broken all-six-explicit form was affected and this is the experimental @orpc/experimental-effect package, that tradeoff is reasonable; a release-note mention is the only follow-up worth considering.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Re-reviewed head at
9ab23591d— this is a force-pushed/amended commit whose diff is byte-identical to the previously reviewed5eb68c0e3(both changed files match exactly, same hunks, same line numbers). No new substantive changes; the earlier approval stands.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Added zero-argument
middlewareGentype test —middleware.test-d.tsaddsinfers everything without type arguments when created via os.middleware, coveringmiddlewareGen(...)with no type arguments insideos.$context<ServerContext>().middleware(...), then.used on a procedure with concrete input/output, asserting context narrowing flows through to the handler.
I verified the new test genuinely guards the fix rather than being decorative: with middleware.ts reverted to the old no-defaults signature, the root pnpm type:check (which is what exercises *.test-d.ts) fails at middleware.test-d.ts(126,12) — TOutput/TInput collapse to unknown and .use(requireAuth) no longer assigns. So it is real regression coverage; that it also fails under this repo's tsc harness makes it stronger than the PR description implies (which attributes the failure only to a TypeScript 7 native compiler). Full type check and lint are clean at head.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
More templates
@orpc/ai-sdk
@orpc/arktype
@orpc/bun
@orpc/client
@orpc/cloudflare
@orpc/contract
@orpc/experimental-effect
@orpc/evlog
@orpc/hibernation
@orpc/json-schema
@orpc/experimental-msw
@orpc/nest
@orpc/next
@orpc/node
@orpc/openapi
@orpc/opentelemetry
@orpc/pinia-colada
@orpc/pino
@orpc/publisher
@orpc/ratelimit
@orpc/server
@orpc/shared
@orpc/swr
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/zod
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |

A standalone middleware created with
middlewareGencannot be attached to any procedure that has a concrete output:Before this change, the only way to make it compile was to spell out all six type arguments, including a hand-written
any:Core already solves this for vanilla middleware:
Builder.middlewaredeclares$Output = anywith the comment "$Output = any by default is important to make middleware can be used in any output by default" (packages/server/src/builder.ts).middlewareGenhas no defaults, so inference falls back tounknownand the middleware is only usable inline or on builders without input/output schemas — which is exactly what the existing standalone type test covered, so this went unnoticed.This mirrors the core defaults (
TInput = unknown,TOutput = any,TErrorMap = Record<never, never>,TYielddefaulted) and movesTOutContextto the second position, so the same middleware becomes:And when the middleware is created through a builder's
.middleware(), no type arguments are needed at all — the in-context arrives contextually and the narrowed out-context is inferred fromnext({ context: … }):(The zero-argument form compiles under
tsceven without the defaults thanks to context-sensitive inference threading$Output = anyinto the inner call, but the TypeScript 7 native compiler resolves the inner call tounknownon the current signature — with the defaults it works there too. Both forms are covered by the added type tests.)Note the reorder is breaking for callers passing explicit type arguments — but passing all six explicitly was previously the only way to make standalone middleware compile, so the affected code is the code this fixes. If you'd rather avoid the reorder, adding only the defaults (keeping the current order) also fixes compilation; the reorder is what makes the explicit form pleasant.
Adds type tests attaching a standalone generator middleware to a procedure with concrete input/output — via
.use()with explicit type arguments, and via.middleware()with none.Validated against a real consumer (an Effect 4 +
@orpc/experimental-effectAPI on TypeScript 7): the auth-guard middleware in the six-argument form above reduces to the zero/two-argument forms, with the app's full typecheck and test suite passing.