feat!: more flexible koa server creation #88
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mnahkies
force-pushed
the
mn/feat/more-flexibility
branch
from
dd99aa2
to
b666237
Compare
- split out a generated `createRouter` function that only creates the koa router, making use of the `bootstrap` / `startServer` optional - allow passing options to the cors / koa-body middlewares, or disabling these completely - return the koa `app` and the `AddressInfo` from `startServer` / `bootstrap` - make `startServer` and `async` function that rejects if `app.listen` fails **this is a breaking change**, and probably not the last to this interface. whilst the prior iteration was far too inflexible, this doesn't feel quite right as it makes it a little too easy to construct a server that doesn't work (eg: disabling body parser without providing alternative). the bootstrap function is kinda redundant now as well, though that may not remain true so can stay for the moment. **migration:** see integration-tests/typescript-koa/src/petstore-expanded.yaml/index.ts for example
``` TS2742: The inferred type of createRouter cannot be named without a reference to koa-body/node_modules/@types/koa. This is likely not portable. A type annotation is necessary. ```
mnahkies
force-pushed
the
mn/feat/more-flexibility
branch
from
b666237
to
ea6f04e
Compare
mnahkies
force-pushed
the
mn/feat/more-flexibility
branch
from
ea6f04e
to
0826494
Compare
mnahkies
force-pushed
the
mn/feat/more-flexibility
branch
from
3c4f63a
to
5d4d992
Compare
mnahkies
force-pushed
the
mn/feat/more-flexibility
branch
from
5d4d992
to
2910981
Compare
mnahkies
commented
Nov 11, 2023
| app.use(router.allowedMethods()) | ||
| app.use(router.routes()) | ||
|
|
||
| return new Promise((resolve, reject) => { |
There was a problem hiding this comment.
Pretty ugly use of new Promise and several try-catch blocks, but whilst I don't think sever.address() throws right now, there's a FIXME comment suggesting it might in future, and doing this means that any exceptions will definitely stay in the promise chain correctly.
Also starts handling errors like EADDRINUSE properly, as these get emitted as an error event
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
split out a generated
createRouterfunction that only createsthe koa router, making use of the
bootstrap/startServerfunctions optional
allow passing options to the
cors/koa-bodymiddlewares, ordisabling these completely
support passing a
ListenOptionsinstead ofportnumber, allowingto bind to a specific network interface
return the koa
appand theAddressInfofromstartServer/bootstrapmake
startServeranasyncfunction that rejects ifapp.listenfails
this is a breaking change, and probably not the last to this interface.
whilst the prior iteration was far too inflexible, this doesn't feel quite right still:
startServerstill pretty inflexible/simple, and doesn't support all optionsbody: "disable"and no suitable alternative providedbootstrapfunction basically redundant now as well, though that may not remain true so can stay for the momentcloses #94