I think I agree with pretty much all of this, Mikeal. Exposing ctors
really invites the wrong kind of composability, but we need to make
the core systems nicer in the ways you describe. The response header
handling really needs to be cleaned up. It mostly works, but has some
odd edges where it doesn't.
Constraints:
1. We must maintain backwards compatibility unless it is completely
unreasonable to do so. This can often be done by re-implementing old
API surfaces in terms of new ones, but please bear this in mind for
any change.
2. API cleanup should generally not introduce new features, but merely
make existing features more intuitive, organized, and consistent.
3. Performance regressions cannot be tolerated. Express or request or
fstream can do a bunch of extra junk to make a cuter API, and give up
some performance in doing so, but node-core must be as fast as
possible.
Here's a half-thought-out proposal. Please shoot it down in
constructive ways. :)
Expose the actual headers as they will be written. This must be a 2d
array. An object is not the right data structure, because there can
be multiple values for the same key, and though it shouldn't, order
does matter in some very rare edge cases. Like module.exports, if
this value is overwritten, or manually modified, then that's the
"real" header.
response.headers = [ [key, value], ... ]
Sugar around response.headers.push([key, value]), but giving us a
place to add intelligent defaults, validation, etc. We should push
this api everywhere as the default:
response.setHeader("key", "value") // Set a header, adding a second
one if there's already one by that name.
response.setHeader("key", "value", true) // Overwrite any existing
header by that name.
Set the status as a property:
response.status = 200; // should this just default to 200?
Backwards-compatible shim method:
response.writeHead = function (status, headers) {
this.status = status;
if (Array.isArray(headers)) {
this.headers = headers;
} else {
Object.keys(headers).forEach(function (k) {
this.setHeader(k, headers[k]);
});
}
};
Don't send the actual header on the socket until the first .write() or
.end(). At that time, response.headerSent = true, and any calls to
response.setHeader() trigger an error event.
On Wed, Feb 8, 2012 at 13:39, tjholowaychuk <[email protected]> wrote:
> Yeah, unless you're one of us writing these higher-level things you
> probably wont even really be concerned about how node does things,
> since if you're just using straight-up node you're probably happy with
> writeHead() etc. It's awesome having a low-level core that is not
> overly opinionated about this sort of thing but at the same time we
> almost need that, otherwise the community just becomes a diverged
> mess.
>
> It's probably not worth talking about rack-like stuff since that'll
> never be in core, but I guess the key thing is that we shouldn't need
> something like that to build pluggable things. Say instead of manually
> checking accept-encoding all over you want:
>
> http.createServer(function(req, res){
> acceptEncoding(req, res);
> // go on with regular stuff here
> });
>
> IMHO we should at least supply something reasonable to make that
> happen. In Connect I'm using a "header" event, it's not as "clean" as
> something like rack but like I've said before I really dont want to
> diverge from what core gives us otherwise it renders that stuff
> useless to other people (like it does now) and splits the community
> more than it should
>
> On Feb 8, 1:35 pm, Mikeal Rogers <[email protected]> wrote:
>> Yup, none of this is new.
>>
>> I think that we need to reach a consensus that this is our goal. I know that
>> in core there is a tendency to hold internal state and refrain from
>> exposure. I think that some people believe it is "safer" or more "secure"
>> and you can see this show a little in the thread about exposing core
>> constructors.
>>
>> A pre-requisite to me writing patches that solve this is a cultural
>> consensus that we are working towards a composable core, and while I know TJ
>> is all for it :) I don't think this goal is currently shared across the core
>> community and the practice to date seems to lead towards writing things
>> initially very restrictive initially and then people like TJ and Marco
>> fighting to expose them.
>>
>> On Feb 8, 2012, at February 8, 20124:18 PM, tjholowaychuk wrote:
>>
>>
>>
>>
>>
>>
>>
>> > +1 been saying this for years
>>
>> > On Feb 8, 1:04 pm, Mikeal Rogers <[email protected]> wrote:
>> >> All the streams in core are in some half state between declarative and
>> >> composable.
>>
>> >> http.ServerResponse is probably the biggest/worst example where:
>>
>> >> http.writeHead(status, headers)
>>
>> >> can, in some cases, entirely opt out of the composable headers set:
>>
>> >> http.setHeader(name, value)
>>
>> >> in particular, this happens when you pass an array of tuples as the
>> >> headers argument to writeHead.
>>
>> >> Then there is http.ClientRequest, which is not composable at all and has
>> >> no setHeader() method.
>>
>> >> I have a similar problem with every stream in core except sockets. All of
>> >> the streams in core take, at some point, a set of parameters that set a
>> >> bunch of internal state and *start* an IO operation. Until that IO
>> >> operation hits a particular event, in the future, none of that internal
>> >> state is actually used. Some of that state can be mutated (is
>> >> composable), some currently isn't (is not composable), and some of it is
>> >> mutable if you access internal properties (ewwww).
>>
>> >> If the goal of core is to reach API stability we need to get core to a
>> >> place where it's extensible. There is a similar thread to this effect
>> >> about exposing core constructors, but I'd like to stay on track in this
>> >> thread and just talk about composability because it doesn't have many of
>> >> the same concerns people have about exposing core constructors.
>>
>> >> *Most* of the objects in core are accessed by userland as the subject of
>> >> an event. When those objects are not composable we incredibly limit the
>> >> extensibility of core, even when we expose the constructors, and it leads
>> >> to the worst kind of monkey patching. There are even cases where core
>> >> objects that get exposed by libraries have certain methods entirely
>> >> removed because they break composability (I know TJ was considering
>> >> removing writeHead when exposed by Express).
>>
>> >> A few things would need to happen to make core more composable.
>>
>> >> 1) Internal state needs to become external state.
>> >> and
>> >> 2) The removal, or "re-integration", of methods that currently break
>> >> composability.
>>
>> >> We may also need to add more events to some objects. Some good examples,
>> >> which have been added, are the 'open' and 'socket' methods on
>> >> fs.ReadStream/fs.WriteStream and http.ServerResponse.
>>
>> >> Anywhere we can expose internal state publicly we increase composability
>> >> because we allow new mutations that depend on that state the way internal
>> >> logic depends on it. As we do this we'll also realize more places where
>> >> we break that mutability.
>>
>> >> Thoughts?
