-
Notifications
You must be signed in to change notification settings - Fork 29
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
request/response objects do not implement the entire core req/res API #4
Comments
Giving it a real compatibility layer would kinda bloat the code. -1 to that. |
I would +1 "better" backwards compatibility, since:
|
Couldn't this compatibility layer be implemented as a middleware, and you could do :
thus not bloating the code ? |
@dready92 +1 to that, that's a great idea! |
I think it's a reasonable way to hack in what you need, but I do wish it didn't have to be permanent. Maybe a compatability "wrapper" would be better:
If this was done "right", then expressCompat would return a non-punched request and response object, but expose punched objects to someExpressPlugin. Edit: A permanent monkey puncher would be a good start. |
@jesusabdullah +1 to |
This does not have anything to do with express or connect compatibility. It is simply that the APIs currently implemented:
The
A pull-request implementing these methods on the core |
An implementation note: https://github.com/senchalabs/connect/blob/master/lib/patch.js#L39-65 Also, I have no intention of backwards compatibility with all of the other |
For reference, by those I mean: https://github.com/visionmedia/express/blob/master/lib/request.js |
@indexzero Good to know! I have to ask though: Why didn't we extend the base object the same as connect? |
That is how we get streaming and non-streaming support. |
So yeah, I made a branch that implements these! I haven't tested them though, so there might be some missing links. |
This is fixed in |
I tried to use connect's static provider, which should've Just Worked. However, it seems that the req and res objects between connect/express and flatiron are not 100% compatible.
Here's a reproducing test program:
This program works fine for the express server, but for the union server:
The text was updated successfully, but these errors were encountered: