-
Notifications
You must be signed in to change notification settings - Fork 1.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
feat(http-server): make http2 server creation extensible #2078
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,7 @@ | ||
// Copyright IBM Corp. 2017,2018. All Rights Reserved. | ||
// Node module: @loopback/http-server | ||
// This file is licensed under the MIT License. | ||
// License text available at https://opensource.org/licenses/MIT | ||
|
||
export * from './types'; | ||
export * from './http-server'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
import {AddressInfo} from 'net'; | ||
import * as http from 'http'; | ||
import * as https from 'https'; | ||
import {IncomingMessage, ServerResponse} from 'http'; | ||
|
||
// Copyright IBM Corp. 2018. All Rights Reserved. | ||
// Node module: @loopback/http-server | ||
// This file is licensed under the MIT License. | ||
// License text available at https://opensource.org/licenses/MIT | ||
|
||
export type RequestListener = ( | ||
req: IncomingMessage, | ||
res: ServerResponse, | ||
) => void; | ||
|
||
/** | ||
* Basic HTTP server listener options | ||
* | ||
* @export | ||
* @interface ListenerOptions | ||
*/ | ||
export interface ListenerOptions { | ||
host?: string; | ||
port?: number; | ||
} | ||
|
||
/** | ||
* HTTP server options | ||
* | ||
* @export | ||
* @interface HttpOptions | ||
*/ | ||
export interface HttpOptions extends ListenerOptions { | ||
protocol?: 'http'; | ||
} | ||
|
||
/** | ||
* HTTPS server options | ||
* | ||
* @export | ||
* @interface HttpsOptions | ||
*/ | ||
export interface HttpsOptions extends ListenerOptions, https.ServerOptions { | ||
protocol: 'https'; | ||
} | ||
|
||
/** | ||
* HTTP/2 server options | ||
* | ||
* @export | ||
* @interface Http2Options | ||
*/ | ||
export interface Http2Options extends ListenerOptions { | ||
protocol: 'http2'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The built-in |
||
// Other options for a module like https://github.com/spdy-http2/node-spdy | ||
[name: string]: unknown; | ||
} | ||
|
||
/** | ||
* Possible server options | ||
* | ||
* @export | ||
* @type HttpServerOptions | ||
*/ | ||
export type HttpServerOptions = HttpOptions | HttpsOptions | Http2Options; | ||
|
||
/** | ||
* Supported protocols | ||
* | ||
* @export | ||
* @type HttpProtocol | ||
*/ | ||
export type HttpProtocol = 'http' | 'https' | 'http2'; | ||
|
||
/** | ||
* HTTP / HTTPS server used by LoopBack's RestServer | ||
* | ||
* @export | ||
* @class HttpServer | ||
*/ | ||
export interface HttpServer { | ||
readonly server: http.Server | https.Server; | ||
readonly protocol: string; | ||
readonly port: number; | ||
readonly host: string | undefined; | ||
readonly url: string; | ||
readonly listening: boolean; | ||
readonly address: AddressInfo | undefined; | ||
|
||
start(): Promise<void>; | ||
stop(): Promise<void>; | ||
} | ||
|
||
export interface ProtocolServer { | ||
server: http.Server | https.Server; | ||
/** | ||
* Scheme for the URL | ||
*/ | ||
urlScheme: string; | ||
} | ||
|
||
export interface ProtocolServerFactory { | ||
supports(protocol: string, serverOptions: HttpServerOptions): boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting. The good part about this design is that it follows what we have already in place for body parsers. However, I feel it's unnecessarily complicated for the case of http servers. For long term, I think we need to provide |
||
|
||
createServer( | ||
protocol: string, | ||
requestListener: RequestListener, | ||
serverOptions: HttpServerOptions, | ||
): ProtocolServer; | ||
} |
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.
What's the impact of this change on possible users of
@loopback/http-server
?For example, the following code creates a fully working HttpServer class in the current version (before your changes):
IMO, we should preserve backwards compatibility in our API. Otherwise a new major version must be published, plus we will have to keep maintaining the old version according to our LTS policy...
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.
We can release
@loopback/[email protected]
if we have to break the compatibility due to the nature of this work.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.
Fair enough. In that case:
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.
See also #2103