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

Add optional error parameter to listencallback #163

Closed

Conversation

kapseliboi
Copy link

There seems to be a clear conflict with the typings and the documentation. I have already created an issue about this on Sapper template repository (sveltejs/sapper-template#311) but after a more detailed look it seems that the root reason is caused by this typing error.

In README.md example it shows that it is possible to pass an error parameter to the listencallback but the current version of types doesn't have that. This PR adds that but I am not sure if the type I gave it is correct at all. I copied it from other error handlers.

@kapseliboi
Copy link
Author

kapseliboi commented May 22, 2021

There is also other TypeScript errors with polka().use(compression()) for example. I can try to sort it out as well.

EDIT: It seems that your Request interface is not completely compatible with Express' request. I'm not sure how that should be fixed.

EDIT2: Express's Request contains a 25 properties that your Request doesn't have (at least according to types). Express's Request definition: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/96bddc7af68f2421c9284e98aebb18037d898e9e/types/express-serve-static-core/index.d.ts#L297

@lukeed
Copy link
Owner

lukeed commented May 22, 2021

Hey,

Thank you, but server.listen, which maps to net.listen does not have an error callback. This is/was a README issue. Was news to me too a while back because I definitely remember reading documentation and guides about http.createServer().listen() taking an err callback...but it never existed. It doesn't exist in Express either, but a lot of guides/examples show that to be the case still too.

As for the request types – you can always import the @types/express package and try to use its Request interface. It'll (very) likely still fail and it'd be confusing at best. You're always going to get type errors because TS is looking for exact interface matches, made "worse" by the fact the express.Request is a generic.

Polka's signatures are compatible. Its req is a subset of Express' and its res is always the raw http.ServerResponse – both of which don't match Express' interfaces (and shouldn't, unless you truly need all that). AKA, types won't match.

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