sveltejs / sapper

The next small thing in web development, powered by Svelte
https://sapper.svelte.dev
MIT License
7k stars 434 forks source link

Make type definitions compatible with Express and Polka #1710

Closed benmccann closed 3 years ago

benmccann commented 3 years ago

Closes #1706

The type definition for SapperRequest and SapperResponse is totally borked. It won't work with Express or Polka. This changes it to use the Express types instead which work on both.

I ignored several issues in the codebase as a result of this change. That's probably not the best solution, but everything's been working just fine and it's not worth investing more time with Sapper eventually going away. My priority is just getting Sapper unbroken

I left the SapperRequest and SapperResponse types instead of replacing them with the Express types in order to maintain backwards compatibility

ehrencrona commented 3 years ago

I took a look at this too and produced #1713 with somewhat more specific changes, but I guess either solution is fine.

It's bizarre there was a property declared (search) that just doesn't exist. I seem to somehow have introduced it in #1604. I guess it does speak to your argument that typings tend to drift away from reality unless they checked as part of the build.