refactor: rename FreshContext.req to FreshContext.request

[timreichen]

This PR renames FreshContext.req property to FreshContext.request
This PR adds FreshContext.request property as an alias to FreshContext.req. See #2363 (comment)

[timreichen]

@marvinhagemeister Ready for review.

[marvinhagemeister]

At Deno here we prefer ctx.req for brevity reasons. Merging this would make my coworkers unhappy. Instead of removing it entirely, you could add a getter to alias ctx.request -> ctx.req.

[timreichen]

At Deno here we prefer ctx.req for brevity reasons. Merging this would make my coworkers unhappy. Instead of removing it entirely, you could add a getter to alias ctx.request -> ctx.req.

Are you sure? It seems there are lot of upvotes which agree with the change (#2363 (comment)
I would find it weird to have a property alias just for convenience in a new public API.

[Atmos4]

@timreichen I don't find req weird personally

Because let's be real, this PR is only nitpicking and personal preference. Deep down it's not important what the name of the variable is as long as it's somewhat clear.

I use Hono a lot, it's req in Hono, it saves both maintainers and lib users a few characters, everyone knows what it is, everyone is happy.

Apart from you apparently.

[timreichen]

@timreichen I don't find req weird personally

Because let's be real, this PR is only nitpicking and personal preference. Deep down it's not important what the name of the variable is as long as it's somewhat clear.

I use Hono a lot, it's req in Hono, it saves both maintainers and lib users a few characters, everyone knows what it is, everyone is happy.

Apart from you apparently.

I don't find req weird per se, I would find it weird to have both request and req in an interface when they represent the exact same as proposed by @marvinhagemeister.

But this is not about personal preference, it is about consistency and alignment with web apis.
As I pointed out, request as a property is present in FetchEvent.prototype.request. Naming a property req instead request that represents a Request simply makes it inconsistent with web apis.

What I find would be a personal preference however is to sacrifice consistency over the convenience of saving a few chars.

[timreichen]

@marvinhagemeister I think we should resolve this issue soon. I worked a lot on deno @std for the last two years helping to make the mods consistent among each other and follow web api syntax and conventions. So this issue seems like a no-brainer to me. Is there any chance it can be adopted?

[nnmrts]

Apart from the consistency concerns, I think "req" instead of "request" is also bad because:

• Other, especially older frameworks doing the same does not mean fresh should do it. Quite the opposite actually: future developers born today will not grow up scrolling through express.js docs like we did, so it becomes increasingly likely they are not familiar with the usual req, res or reg, ctx scheme.
• Without enough context (and not "ctx" 😜) it could be confused with the common "require", "requirement" or for people with bad eyes (like me) even with words starting with "reg" like "register", "regular" or "region".
• Without it being the full word, it's unclear which form of the word it is: "request", "requests", "requesting", "requested", etc. are all common variations of that word, affecting the understanding of the actual format of the value behind the variable. Is it an array of requests, or maybe even a boolean?
• It sets a precedent in code bases; let's also call it "res" instead of "response" (which a lot of frameworks sadly do as well, I know), now "res" could also mean the common words "resource", "resolution", "reserve" or even "research". And why not call it "dep" instead of "dependency" (also common, sadly), which could stand for a bunch of common words too?
• There are abbreviations which actually save time typing and maybe even reading and understanding code, for example "fs" instead of "file system" or the more cryptic "i18n" instead of "internationalization", but in this case, it's 3 instead of 7 letters. We are not saving any "real" time here, and also, by the time you typed the "q", your (hopefully modern) IDE will autosuggest or -complete it to "request" anyway.
• "Using complete words results in more readable code. Not everyone knows all your abbreviations. Code is written only once, but read many times." - quoted from a documentation of an eslint rule I personally use (with great success 😉).

In many of the points above, I speak from personal experience. On several occasions, I’ve had to remind work colleagues, non-technical product owners, or even my own tired 3am brain what "req", "res", "opt", "dep", "acc", etc. mean at some line deep in a codebase. And that is where you lose actual time. In our newer codebases, I introduced the aforementioned eslint plugin and code reviews are now 20% more fun (a made up stat, but I hope you get my point).

[timreichen]

@marvinhagemeister I added the alias with a deprecation note. I think that way a slow adoption can happen during the alpha releases and before 1.0 is released. WDYT?

[justinmchase]

Yes please, also response should be on the context not returned from the next function.

[timreichen]

Yes please, also response should be on the context not returned from the next function.

Was this discussed somewhere? I think this might be better in a separate issue/PR.

[timreichen]

@marvinhagemeister Would it be possible to reevaluate this PR? I hope I don't have to rebase it any longer:)

[nnmrts]

😭