feat: add Props type parameter to ExportedHandler for typed ctx.props#784
feat: add Props type parameter to ExportedHandler for typed ctx.props#784HueCodes wants to merge 7 commits intocloudflare:mainfrom
Conversation
Add a Props type parameter to ExportedHandler so that ctx.props can be properly typed when using `satisfies ExportedHandler<Env, Props>`. - Add enhanced ExportedHandler<Env, Props, QueueHandlerMessage, CfHostMetadata> - Add Props-aware handler types (ExportedHandlerFetchHandler, etc.) - Export types from main entry point and agents/types - Add generic params to createMcpHandler and McpAgent.serve() Fixes cloudflare#501
🦋 Changeset detectedLatest commit: 88de792 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
whoiskatrin
requested review from
deathbyknowledge,
mattzcarey and
threepointone
commit: |
|
I'll need to review this |
deathbyknowledge
left a comment
deathbyknowledge
left a comment
There was a problem hiding this comment.
Let's keep the changes in in /mcp but remove the rest
| * Defaults to Streamable HTTP transport. | ||
| */ | ||
| static serve( | ||
| static serve<Env = unknown, Props = unknown>( |
|
Thanks for the review! I understand now that I was unnecessarily re inventing types that already exist. Will fix this later today. Really appreciate the feedback |
- Update MCP handler type constraints per reviewer suggestion: - Env extends Cloudflare.Env = Cloudflare.Env - Props extends Record<string, unknown> = Record<string, unknown> - Remove ExportedHandler types (will be added to workerd repo instead) - Update tests to work with new type constraints - Update changeset description
threepointone
left a comment
threepointone
left a comment
There was a problem hiding this comment.
needs fixes to pass CI
Remove overly strict type constraints that required Env to extend Cloudflare.Env and Props to extend Record<string, unknown>. This allows users to pass ExecutionContext without explicit type parameters. Fixes TypeScript errors in examples that use `env: unknown` or `ExecutionContext` without type parameters.
|
CI failures are unrelated infrastructure issues (missing |
| Env extends Cloudflare.Env = Cloudflare.Env, | ||
| Props extends Record<string, unknown> = Record<string, unknown> | ||
| >( | ||
| export function createMcpHandler<Env = unknown, Props = unknown>( |
There was a problem hiding this comment.
hmm I preferred this to default to Cloudflare.Env, why was it necessary to change it back to unknown? trouble with typescript?
There was a problem hiding this comment.
I went with unknown thinking it would be better for type safety and was trying to make it more flexible. So you didn’t need to use Cloudflare.Env and was thinking they’d be unknown until the user specifies. What is your preference? Looking back now I can see why you’d specify in a cloudflare repo.
5 participants
Add a Props type parameter to ExportedHandler so that ctx.props can be properly typed when using
satisfies ExportedHandler<Env, Props>.Fixes #501