On Feb 8, 2012, at February 8, 20126:07 PM, Isaac Schlueter wrote: > 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.
I would entirely agree with this except the fact that I know the code where our current writeHead bugs are that break composability and they are in optimized that landed over a year ago. > > Here's a half-thought-out proposal. Please shoot it down in > constructive ways. :) > I have a mostly-thought-out proposal https://github.com/joyent/node/issues/1448 I mainly wanted to get buy in among core that this is a goal and we should err on the side of exposing previously private state as well as composability before I dug in to fixing things. > > 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 <tjholoway...@gmail.com> 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 <mikeal.rog...@gmail.com> 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 <mikeal.rog...@gmail.com> 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?
