-
-
Notifications
You must be signed in to change notification settings - Fork 17.1k
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
Fix inheritance should not overwrite the methods on res and req with the ones from http.ServerResponse and http.IncomingMessage #6047
base: master
Are you sure you want to change the base?
Conversation
…se and http.IncomingMessage
Unfortunately I think we are lacking a few things on this PR:
If you are interested in working toward this same goal then I would love to chat more on this. I am just not sure I see how this specific PR fits in with the plans. |
Yes this is what is outlined in #6039 The reason for using unenv is just to be able to run SSR requests/fetch on the same server without a network roundtrip. Basically on nuxt SSR, unenv is there so that when you fetch locally a mocked Request is sent to express. The problem this is solving is the use of As such if you have a response or request object not originally coming from node, but simply meant to have a compatible api (like unenv) the methods of the unenv class will be replaced with the methods of the node class but the internals of the class very much differ, resulting in errors like with This PR fixes that by running a modified setPrototypeOf that will only replace all the non native methods of the class (so what express is adding and the users are adding, but not the ones from the original prototype) I do not get your point about this PR needing cleanup, it's a pretty straightforward PR with minimal changes for a problem that took me weeks if not months to track down and solve efficiently without breaking anything in express. It has clearly already been cleaned up
Express is being considered legacy because of those design issues. Meaning if I do want to use express on h3/unenv, it's because I need a few endpoints coming from an (oldish) library built for express. Without having to redevelop them or having to wait for it to become compatible with express v6 which would likely take a few years after v6 comes out as it is still on express v4. So while this is really great news, it's already a bit late and v5 which is pretty recent should get some attention to make it compatible with other envs if it's doable minimally and without breaking change like here |
If you provide some code examples it might help, but I think you can use the https://github.com/expressjs/express/blob/master/lib/express.js#L45-L52 |
This allows for express to run correctly with
unenv
and paves the way for it to be used in other envs thannode
Fixes #6039
I also have a backport for 4.x ready