Skip to content
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

fix: context type for CompatibilityRequestProps #164

Merged
merged 1 commit into from
Aug 23, 2022
Merged

fix: context type for CompatibilityRequestProps #164

merged 1 commit into from
Aug 23, 2022

Conversation

sisou
Copy link
Contributor

@sisou sisou commented Aug 15, 2022

The changes in PR #124 to make the H3EventContext extendable by userland types were insufficient to apply the type also to the CompatibilityEvent, which is the type of events for event handlers. I always got context.user: any when only declaring the context interface:

declare module 'h3' {
	interface H3EventContext {
		user: User,
	}
}

Because CompatibilityEvent can also be an IncomingMessage, which extends the CompatibilityRequestProps, which in turn still declares an un-extendable context: Record<string, any>. This gives CompatibilityEvent.context: H3EventContext | Record<string, any>, which gives any for all properties, even if H3EventContext is typed.

The changes to make the context extendable by userland types in PR #124 were insufficient to apply the type also to the `CompatibilityEvent`.
@pi0 pi0 changed the title Fix event context in CompatibilityRequestProps fix: fix context type for CompatibilityRequestProps Aug 23, 2022
@pi0 pi0 changed the title fix: fix context type for CompatibilityRequestProps fix: context type for CompatibilityRequestProps Aug 23, 2022
@pi0
Copy link
Member

pi0 commented Aug 23, 2022

Thanks!

@pi0 pi0 merged commit 984a42b into unjs:main Aug 23, 2022
@sisou sisou deleted the patch-1 branch August 23, 2022 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants