Deprecate req.path and replace with req.pathname

[dougwilson]

So currently in Express 4 (and lower) there is req.path, which is added by Express. This property will return the pathname of the given request. There has surfaced two issues here now:

(1) The new Node.js HTTP/2 compatibility API introduces a new (currently undocumented) req.path property that is an alias for the :path puesdo-header (req.headers[':path']) and so the meaning of req.path in Express vs in the HTTP/2 compatibility layer differ (Express = pathname of req.url and HTTP/2 compatibility API = :path header, basically what req.url is in HTTP/1). (2) The property itself is sort-of misnamed, similar to the req.host header that was deprecated already and replaced with req.hostname (to come back in Express 5.0 with the current meaning in req.host).

I am proposing that req.path is deprecated in Express 4 with a new req.pathname introduced in Express 4 and beyond to hold the existing req.path behavior.

[wesleytodd]

Is the plan to fully remove the old functionality in 5.x, and add a deprecation warning and the new property in the next minor release of 4.x?

The only concern I have with that is that this is similar to what happened with the zero fill buffer issue. Once everyone started seeing the deprecation warnings they opened issues, causing a ton of churn and work for maintainers. I know we are on a slower release cycle, so it makes it harder because we can't just wait for 6.x, but I just want to avoid this if possible. Especially since this one is very widely used in applications.

[dougwilson]

There isn't reallt a set plan right now, which is why I opened to discuss :) As it stands, this property is one issue blocking using the new code HTTP/2 stuff. Ideally it should be marked deprecated in the 4.x line, but another option is no provide no warnings and assume folks will throughly read the 4 to 5 migration docs to figure it out. I'm all ears for any ideas here :)

[wesleytodd]

I think any churn/work for maintainers of core, in this case, is worth getting the http2 stuff into 5. Sadly...

assume folks will throughly read the 4 to 5 migration docs

is just not feasible for all the people with packages in the ecosystem. Having dealt with both the zero-fill buffer and the react 15.5 issues, I think that we are setting ourselves up for problems if we print deprecation warnings in the 4.x line for such a relied upon feature. What do you think about cutting the 5.0 branch early, and getting 6.0 support for http2? I know it is not desirable, but just want to make sure we leave "no stone unturned" before subjecting every package maintainer of an express middleware to these issues.

At this point, many of the merged breaking changes are stable(ish?), the rest that are pending are smaller. After 5.x, maybe we can plan on dropping 6.0 sooner rather than later with more of the larger changes?

[dougwilson]

Right. But how long would people expect the 4.x and 5.x line be maintained? Would this mean actively maintaining 3 releases for a long time?

[dougwilson]

Sorry, to complete my thought: since these packages likely don't take a dependency on express and instead are subject to the version of Express the user is using, I'm not sure shipping a new major would alleviate this particular issue, since users will still upgrade and file issues everywhere, unless I'm missing something I guess.

I am vety aware of deprecation issues and a package maintainer myself, and certainly want them to be seeiously warranted before subjectinf anyone to them. I also know that users think they are "errors" and not simply warnings that don't break anything.

At least where I am, in thr corporate world, Express is really looked up to for the extremely long support timelines of major versions. Node.js is sadly being hurt with the frequent major release, unfortunately. There isn't time to constantly replatform every year, lol.

[wesleytodd]

Even if the intermediary packages don't directly depend on express, they would still trigger the deprecation warning by accessing req.path. If you published 5.x with the warning, and also started publicizing that support for 4.x will end soon, then people would start migrating to the new req.pathname implementation now.

The people who are ready for it can adopt 5, change to pathname, and move along. People who are not will stop receiving updates other than if we decided to still do severe security patches (which is optional). This would move express forward without disrupting EVERYONE by printing a dep warning in a minor point release.

I think it is reasonable for to pick a support window, even if it is a shorter one. As long as it is plastered up on the website and readme so people updating will know what to expect. Maybe to satisfy the corporate need for long support timelines, we could support 5 for a longer period?

I know what you are saying with node upping its pace being a problem in many areas, but one thing they are doing well is having a documented support timeline. Maybe that is something that express should do?

[dougwilson]

I've been telling people so far (even on here) that Express 4 will be supported for at least 1 year after Express 5 comes out.

[dougwilson]

We could maybe just add req.pathname in 4.16 just deprecating it in docs. Then maybe in 6 months release a 4.17 with it deprecated in code? Does that seem viable in some way?

The people who are ready for it can adopt 5, change to pathname, and move along.

I thought your argument here was module authors; the authors cannot control the version the users use, so people will still plaster issues across the modules even if the warning didn't appear until 5.0.

[wesleytodd]

Yep, these kind of changes are never fun/easy. Maintaining 4, 5 and a possible 6 for a year does not sound like a great idea. Maybe the best overall solution is just printing the warning in 4.x. But at least if anyone takes issue with it we can point to this conversation to make it clear it was taken seriously.

I think the gradual idea is better than nothing, it would hopefully make it less of a "hair on fire" issue where it all happens at once. I did originally focus on module authors, but the more I actually think about it, the concern is equally module authors and application developers.

[dougwilson]

Right, the concern is indeed everyone, but usually the module authors are hit a bit harder when it is a change from something they don't have a direct version dependency on (like Node.js, Express.js) so they are really off guard. The app folks are usually a bit less, sonce they are the folks actually choosing the version (or choosing to live with a giant version range).

If we decide to move forward on this, we could possible try out of band warnings as the first stage. In other wards, add req.pathname just as another API in 4.16 maybe and then warn about it only in human land (the website announccement bubble, API docs, maybe even PRs to popular modules / ones you personally use) to either use

require('parseurl')(req).pathname

or something verbose like

var pathname = req.pathname
if (typeof pathname !== 'string') pathname = req.path

Or something. Certainly want to figure out a good solution. I did just read a thread that looks like Node.js master branch has removed req.path from the http/2 compat layer, though it is not in any Node.js release yet.

[dougwilson]

So I opened the 4.16 PR, and it sounds like at least for here, we should just add a req.pathname property as an alias but leave req.path alone for now, right?