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

[FEAT] Expose proxy Request class for public interface #26

Closed asos-craigmorten closed 3 years ago

asos-craigmorten commented 4 years ago

Issue

Currently the Opine Request object is a mutated version of the Deno ServerRequest. This refactor is to instead create a new Request class that is provided to the user, whose methods / properties proxy to the underlying ServerRequest.

Details

As the body property of ServerRequest is read-only it leads to a jarring interface when Opine body-parsers have to place parsed bodies onto a parsedBody property rather than onto body itself. This deviates from Express, which though isn't necessarily a bad thing, given the goal of initially mirroring the API as close as possible for v1 it is certainly something that should be achievable easily enough.

Initial thoughts are to create a new Request class which contains the custom Opine methods / getters etc. and which also supports the ServerRequest interface with at least one difference: the body property should be either the #parsedBody if exists, or fallback to the ServerRequest body or rawBody if not.

As a rough guide:

class Request {
  parsedBody: string; // TBC
  rawBody: Deno.reader | UInt8Array | Buffer | string; // TBC

  constructor(request: ServerRequest) {
    this.rawBody = request.body;
    // ...
  }

  get body() {
    return this.parsedBody ?? this.rawBody;
  }
}
JayHelton commented 4 years ago

Hello! I am very interested in contributing to opine. I am a long time express user and have recently become smitten with deno. There are no features or bugs that I currently have my eyes on (so im totally open to just grabbing this). I would love to talk with you guys to see where I can help out!

asos-craigmorten commented 4 years ago

@JayHelton that's awesome, any help, suggestions etc. etc. are very welcome.

The initial motivation of Opine was to provide a server / middleware option for current Express users that truly honoured the original interface as closely as Deno would sensibly allow so there was / is an easy first step into the Deno userland. Eventually I envisioned we could write up a CLI that could port existing Express projects over to Deno, or make them "universal" (Node and Deno compatible) using Opine APIs as the Deno compatibility layer.

For now the plan is to try and close the feature gap between Opine and Express. For this particular ticket, by aligning the use of req.body within Opine to how it is used and mutated in Express. Namely in Express the various body-parsers and the user are able to mutate / overwrite the request body with decoded / parsed content. As-is, this isn't an option with Opine as the ServerRequest class has a read-only body property, so the suggestion is to create a new object / class / proxy which provides the necessary getters / setters needed.

Hopefully between this and the description of the issue it's clear what the proposal is? But do shout if have any questions! 😄


As an aside, if you scan the code you'll find it is littered with TODOs, all of which are free game to pick up and implement / fix. If there's anything you find and fancy tackling then do raise an issue and crack on 🚀 I'll try and write up a few more feature issues for contributions - had a few but folks have done them!