-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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(types)!: expose httpServer with Http2SecureServer union #14834
Conversation
Run & review this pull request in StackBlitz Codeflow. |
This will affect downstream plugins and projects that access the |
/ecosystem-ci run |
📝 Ran ecosystem CI on
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can give this a try. It also looks like we attempted this in the past (#10196) but reverted as it's breaking downstream. Now in the Vite 5 beta would be a good time to go with this.
EDIT: oh it looks like there's merge conflicts
HttpServer
type to avoid type assertion
Looks like new added AsyncDisposable conflicts with this, I've resolved this conflict. |
Description
The
http.Server
andHttp2SecureServer
types were previously asserted to HttpServer directly. This meant that some properties were potentially unusable:vite/packages/vite/src/node/http.ts
Lines 108 to 118 in 9866be0
This PR explicitly declare a
HttpServer
type as the union of the two server types. These two types share a large common property - only a small subset of properties are unavailable when using the unionHttpServer
type.vite/packages/vite/src/node/server/index.ts
Line 185 in 3db374a
This makes property usage more strict and avoids impossible states when using the two server types interchangeably.
Additional context
Some properties are now inaccessible when using
HttpServer
:http.Server only:
Http2SecureServer only:
However, none of these properties were used in the current code. So the behavior impact of this type change is theoretically none.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).