request/response objects do not implement the entire core req/res API

[jfhbrook]

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:

var union = require("union"),
    express = require("express"),
    static = express.static(__dirname + '/static');

union.createServer({
  before: [
    static
  ]
}).listen(8080);

express.createServer(static).listen(8081);

console.log("union: localhost:8080");
console.log("express: localhost:8081");

This program works fine for the express server, but for the union server:

josh@onix:/tmp/express-test$ node test.js 
union: localhost:8080
express: localhost:8081

/tmp/express-test/node_modules/express/node_modules/connect/lib/middleware/static.js:162
    if (!res.getHeader('Date')) res.setHeader('Date', new Date().toUTCString()
             ^
TypeError: Object [object Object] has no method 'getHeader'
    at /tmp/express-test/node_modules/express/node_modules/connect/lib/middleware/static.js:162:14
josh@onix:/tmp/express-test$ 

[mmalecki]

Giving it a real compatibility layer would kinda bloat the code. -1 to that.

[jfhbrook]

I would +1 "better" backwards compatibility, since:

1. It's a major feature of Union.
2. Without work on backwards compatibility even "basic" express plugins bomb (like express.static).

[dready92]

Couldn't this compatibility layer be implemented as a middleware, and you could do :

var expressCompat = require("union/expressCompatibility");
union.createServer({
  before: [
    expressCompat,
    static
  ]
}).listen(8080);

thus not bloating the code ?

[mmalecki]

@dready92 +1 to that, that's a great idea!
@jesusabdullah what do you think?

[jfhbrook]

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:

union.createServer({ before: [ expressCompat(someExpressPlugin), somePluginThatDoesntExpectMonkeyPunching ] });

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.

[mmalecki]

@jesusabdullah +1 to expressCompat. Lets go this way, it can be even easier than permanent one.

[indexzero]

This does not have anything to do with express or connect compatibility. It is simply that the union.ResponseStream (https://github.com/flatiron/union/blob/master/lib/response-stream.js) is not 100% compliant with all core http.ResponseStream APIs.

APIs currently implemented:

• .end()
• .write()
• .writeHead()

The .setHeader and .getHeader APIs you are referring to are not part of connect or express:

• .getHeader(): https://github.com/joyent/node/blob/master/lib/http.js#L542
• .setHeader(): https://github.com/joyent/node/blob/master/lib/http.js#L525

A pull-request implementing these methods on the core union.ResponseStream will be accepted

[indexzero]

An implementation note: connect does not modify the http.ServerRequest or http.ServerResponse objects at all except for a small patch to http.ServerResponse to better support Set-Cookie headers.

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 http.ServerRequest or http.ServerResponse extensions in express.

[indexzero]

For reference, by those I mean:

https://github.com/visionmedia/express/blob/master/lib/request.js
https://github.com/visionmedia/express/blob/master/lib/response.js

[jfhbrook]

@indexzero Good to know! I have to ask though: Why didn't we extend the base object the same as connect?

[indexzero]

That is how we get streaming and non-streaming support.

[jfhbrook]

So yeah, I made a branch that implements these! I haven't tested them though, so there might be some missing links.

[indexzero]

This is fixed in v0.1.3