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

Move req prototype to separate repo #82

Open
wesleytodd opened this issue Jul 19, 2019 · 8 comments
Open

Move req prototype to separate repo #82

wesleytodd opened this issue Jul 19, 2019 · 8 comments

Comments

@wesleytodd
Copy link
Member

The story started in 2014: expressjs/express#2432

I even made the repo where I intended to do this ~2 years ago, but never finished the work.

Since then I have talked with a few people about how the different frameworks and platforms can collaborate. And then @ianstormtaylor kept tweeting about it, so tonight I took the plunge ;)

Here is the initial repo: https://github.com/wesleytodd/express-request

I wrote a few more thoughts about it in the readme for this repo, but I wanted to add here what I think the plan should be:

  1. Move this repo into the express org. The express org and not pillarjs because this is specifically the Express sugar/compatibility layer. Also, I didn't just create it here because I wanted to get consensus on the direction first.
  2. Branch a v2 where we can make Express 5 compatible updates.
  3. Branch a v3 to work on this being a wrapper for http1/2/3.
  4. Implement individual sugar's in separate repos in pillarjs and integrate them into this allowing for mix-and-match by other implementations. This would be an ongoing process as needed.

The long term goal is that other frameworks and platforms do not need to reinvent the wheel as express evolves. This can also be a place for us to evolve ideas which will ultimately be proposed as additions to the core api's to help improve and evolve them.

If we can get consensus on the api across frameworks and platforms we can then provide shared tools across the framework ecosystems, making the "framework" fungible with the core http features as the shared underlying code.

Other benefits I can think of:

  1. Let framework maintainers focus on what makes their frameworks great
  2. Provide consistency, security and spec compliance to common http layer things
  3. Increase the collaborator base for some of Node's most important modules
  4. Encourage new and innovative http frameworks without fear of the huge overhead involved in all these little things.

Anyway, I just wanted to get the conversion moving with an actual implementation example. So to start I pulled it out, and over the next week or so I will make open a PR against the master/4.0 because this should be backward compatible.

If everyone is onboard with this approach I will do the same for the response and follow the same approach as listed above.

People who might be interested in this to comment on and or collaborate from other frameworks: @matteocollina @styfle @DonutEspresso @hekike

Note: Obviously before we integrate it there is work to do with documentation and getting it fully up to standards with the express org requirements. Removing probably the whole "History and Reasoning" section, sorry Ian 😄

@wesleytodd
Copy link
Member Author

Ok, here is the new request repo. It would be great if folks can take a look before we publish.

https://github.com/pillarjs/request

cc @expressjs/express-tc @expressjs/triagers

@dougwilson
Copy link
Contributor

Awesome @wesleytodd !

I forgot this issue already existed, so I moved my comment from #100 (comment) here to kick off the general conversation.

I think the biggest feedback I have from reviewing them is they... do not actually work independent of each other, yet they are published independently as if they could be. I'm not clear on if the issue is either they they depend on each other by accident or maybe that they should just be a single repo and not two separate repos.

The readme of express-request seems to imply it should be able to be used stand alone, but yet following the example and attempting to run it promptly fails due to there being no this.app defined. Trying to use req.fresh also fails, not just because this.res is not defined, but also because the res is expected to also have been wrapped with the separate package (which it itself has methods expecting req to be wrapped with the other package).

@wesleytodd
Copy link
Member Author

This is a great point! My thought was to copy it over verbatim at first, then discuss what changes we need to make it work outside of Express. I guess I didn't cover that here but my thought is we can move toward it being standalone as we go. Would maybe to discuss this on that repo now that we have it?

@dougwilson
Copy link
Contributor

but my thought is we can move toward it being standalone as we go

Yep, that is what I figured, and I guess my thought(s) above was the start of such a conversation :)

Would maybe to discuss this on that repo now that we have it?

Sure! I will create an issue there 👍

@wesleytodd
Copy link
Member Author

Cool, I have a bunch of thoughts but will reserve them for tomorrow and an issue on that repo. I probably will not get to the res part tonight since we have the web server frameworks meeting at 6am tomorrow, but I will try to get that one knocked out tomorrow.

@dougwilson
Copy link
Contributor

Cool, I have a bunch of thoughts but will reserve them for tomorrow and an issue on that repo.

Definitely. I wasn't even going to open the issue until tomorrow anyhow. There is likely a lot of thoughts all around, haha.

@jonchurch
Copy link
Member

jonchurch commented Mar 30, 2020

Curious if ya'll have thoughts about preserving the git history for req and res? I experimented with git fitler-branch and was able to pull out the files into a new repo while maintaining the entire git history for the selected files.

It's fragile, though, since so far as I understand you need to keep the same filepaths aka lib/request.js as opposed to index.js to keep the history. Eventually the blame would be lost if the file was renamed/moved. But that's no different than normal git history.

@wesleytodd
Copy link
Member Author

Curious if ya'll have thoughts about preserving the git history

I did, but was not sure how valuable it would be, and was planning on reformatting to standardjs formatting which would break blame anyway.

If you want to keep the history and move them you just need to do the pull out first and then move the file in a new commit. Again though, I am not sure how valuable trying to keep it is do didn't try.

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

No branches or pull requests

3 participants