cmorten / opine

Minimalist web framework for Deno ported from ExpressJS.
https://github.com/cmorten/opine/blob/main/.github/API/api.md
MIT License
854 stars 43 forks source link

Add typization for sessions #134

Closed ElikaFilin closed 2 years ago

ElikaFilin commented 3 years ago

Issue

Setup:

Details

Currently adding session into application provokes type error in typescript as Request object in Router doesn't corresponds to the Request object extended by adding session property. As result every place where session used should be marked as Ignored. It is not right behaviour, so Router and Request object should be extensible in easy way

cmorten commented 3 years ago

@ElikaFilin are you able to add any detail to clarify?

If this is regarding session middleware, this is something that can be solved in userland IMO.

Sim. to https://github.com/asos-craigmorten/opine/issues/103

It looks like there may be one already https://deno.land/x/sessions@v1.5.4

asos-craigmorten commented 3 years ago

Closing due to lack of information, and session handling being something that should live in userland.

jcs224 commented 3 years ago

Hi there, just want to chime in since it looks like my fork of the sessions repo has gotten some traction.

@ElikaFilin I see you've forked my session repo and made some nice additions, including bringing back TypeScript support. I'd love to see if there's a way to bring some of those changes back into my project, or, continue contributing to your fork.

Hopefully we can create a good-quality sessions library for Deno, I'm in real need of one :smile:

ElikaFilin commented 3 years ago

@jcs224 You can easily contribute to our repository as we are going to develop this middleware in future

janusz-f commented 3 years ago

@cmorten
currently framework works only with router.get('/', index as unknown as RequestHandler);; but it is not the best solution more like detour

asos-craigmorten commented 3 years ago

Agreed - the typing could do with some work. As a starter for 10 is was derived from the Express types available here --> https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/express/index.d.ts

It would be grand to revisit and improve them to allow extensions such as the session middleware, and fix any bugs with types such as you metion @janusz-f.

Options include updating the https://github.com/asos-craigmorten/opine/blob/main/src/types.ts file, or slowly migrating away from a types file to colocating types with the functionality they describe in the appropriate files.

Welcome any / all improvements if folks want to contribute PRs 😄 🎉

cmorten commented 2 years ago

Closing for now as stale - if still facing issues can happy reopen 😄 contributions also welcome!