Roadmap discussion for 3.0

[naugtur]

Plans for v3.0.0 (updated as discussion progresses)

• 1. drop old IE support, keep it in require('xhr/legacy') or just let people use 2.x
• 2. ensure all options that xhr implements are compatible with request
• 3. add fully automated cross-browser tests (surprisingly hard to do if you need to have a mock server)
• 4. stop accepting content in json option, make it boolean
• 5. fix xhr incompatible with environments that honor object freezing #149

If you have any other important items on your agenda and want to start/keep contributing to xhr feel free to chime in.

[yoshuawuyts]

Sounds good to me! - feel that by dropping old IE support bundle size might go down a little, should it not? - v exciting ✨

[TehShrike]

Seems like good roadmap. No big changes, and should set the library up for long-term stability.

I don't think we should hold off on publishing or merging any of those changes until they're all ready to merge at once, though - it would take a long time, and maybe never happen. Publishing breaking or feature changes on their own isn't a big deal as we follow semver.

If request compatibility matters, #135 should go out sooner rather than later.

We should make a couple stand-alone testing issues and merge pull requests for those as they come in.

[mhart]

When you say "publishing breaking ... changes on their own isn't a big deal" I don't entirely agree. A breaking change means a bump in the major, so 2.x -> 3.x. If you publish two breaking changes (on their own), then you're going to go 2.x -> 3.x -> 4.x

Much better to accumulate known breaking changes until a 3.0 candidate is in pretty good shape and unlikely to change (which I think is what @naugtur is suggesting).

Of course it's fine to merge breaking changes and not publish them – so that master is the 3.0 candidate and create a 2.x branch to be able continue publishing patches and feature additions for the current version.

[TehShrike]

Much better to accumulate known breaking changes until a 3.0 candidate is in pretty good shape and unlikely to change

Why?

I think the downside of having unpublished code is a great reason to publish right away. When each change is published in its own version, it's much easier for users (and maintainers) to figure out which change introduced a bug, if one is released.

Having unreleased code in the repo makes me nervous. Who knows how many months of untested-in-the-real-world code you'll be publishing? It makes publishing a scary proposition, and you don't want to add friction like that on a project like this, that doesn't see probably more than an hour of code time from maintainers every week.

Major version bumps aren't a big deal if you don't have a massive framework-level API. As long as breaking changes are noted in the readme (which they should be anyway, no matter how many version bumps they happen under), I don't see the downside for consumers.

[mhart]

I really don't understand what you're suggesting? Publish #135 as 3.0 right now? And then... this roadmap becomes the 4.0 roadmap? And then the next PR that comes in implementing a request compatible feature next week goes out as 4.0, and then this roadmap becomes the 5.0 roadmap?

I definitely think it's worth waiting on publishing 3.0 at least until a survey of what remaining features of request differ from xhr. I mean, really, the urgency here to publish a major bump seems a bit unfounded.

[TehShrike]

This roadmap becomes the "list of things to be worked on". I think that's a reasonable function for a roadmap.

Can you explain why you place this importance/gravity on 3.0? These numbers are for npm, not for users.

If the API needs to change in a backwards incompatible way to maintain request compatibility, we should make that change, publish as soon as possible, and bump the version number in the package.json accordingly for the benefit of npm. The alternative is that this project languishes with unpublished, needed fixes.

This project isn't big enough for the version number to have any other meaning.

Why wait to publish fixes, except for sentimental reasons?

[mhart]

Again, I disagree that the numbers are not useful for users. Saying "our aim is to be a subset of request by 3.0" is much more useful for planning and much easier for users to understand than "our aim is to be a subset of request at some unknown version in the future"

I'll know that I can install xhr@3.x and the supported subset of options will function in the same way as request. Otherwise, I'll install 3.x knowing that it doesn't necessarily have the same options as request, and that I'll need to constantly check for new major versions, changing my codebase along the way each time, until there is parity.

Would you really be ok with Node bumping the major version every week? That would cause a hell of a lot of churn. Users have a reasonable expectation that they won't need to worry about updating to a new major version every week just to get patches and fixes to their current version because breaking changes are going in every week – unless you're planning on releasing patches for all published major versions? But that would be a hell of a lot of work for maintainers once you're up past 10 major versions.

It's much easier to be able to choose a time to look at a collection of breaking changes and work out how to upgrade your codebase at a minimum every few months than have to worry about doing it every week.

I also disagree that it's a "needed fix" with any sort of urgency – the json option has always been this way in this project. If it's a "needed fix", then every other incompatibility with request is also a "needed fix" and should be fixed immediately! Hey presto, there's v3.0

Why wait to publish fixes, except for sentimental reasons?

You shouldn't! Fixes that aren't breaking changes should be published, I'm in total agreement with that.

If your question is "why wait to publish breaking changes" – then the obvious answer is churn, it's a headache for users, you leave other users behind on unpatched versions. It's much, much better to accumulate a collection of known breaking changes and publish them together – you display much more empathy for your users handling breaking changes like that.

I'm really not talking about anything onerous here – literally just to look at the API and see what else differs besides the json option.

I think this roadmap is a very reasonable goal for the next major version of this library.

[TehShrike]

Saying "our aim is to be a subset of request by 3.0" is much more useful for planning and much easier for users to understand than "our aim is to be a subset of request at some unknown version in the future"

Our aim is to be a subset of request now, and we're not. We should publish changes to resolve this.

Would you really be ok with Node bumping the major version every week?

No - I gave a caveat above, when I said

Major version bumps aren't a big deal if you don't have a massive framework-level API

When your project has many sub-libraries and smaller APIs, managing version releases becomes a much bigger deal. xhr is a single library with a single API, semver is much simpler for us.

Beyond that, node has never properly followed semver. You can read about it on http://sentimentalversioning.org/

New users don't install xhr@3, they install xhr and roll with the latest version, trusting that the readme is accurate.

I'm really not talking about anything onerous here – literally just to look at the API and see what else differs besides the json option.

Being confident in request compatibility requires writing tests for all of the features for request that we want to claim we support.

Even if reading both readmes and writing down notes was all that it would take, and no changes to the code were needed, I wouldn't be getting to that in the next week or two.

This library doesn't have the development resources, or the need, to make a big deal over releases.

[mhart]

@TehShrike I've been a collaborator on this project for more than a couple of years now and this urgency to rush new major versions out just doesn't gel with how they've been handled up until this point.

This library has been stable for a long time now and I don't know why you want to suddenly change that process. It's not like this is a new project – it's been around for years – churn is just unnecessary at this stage of its life.

xhr is used in many mature codebases (I know because I manage them) and the prospect of having to rewrite hundreds of lines because the major version is constantly changing but I want to stay up to date with patches is just not user friendly.

v2.0 wasn't explicitly released so that all options were compatible with request – the handling of error responses were changed to be more request-like with that release – the behaviour of other options like json were not modified to be more request-like during that release.

I think v3.0 is a perfect candidate to explicitly aim to be API compatible (as much as a subset makes sense to be) – I think it's a little unfortunate that the README was changed recently, mid cycle, to say it "is" a subset of request when that was clearly incorrect with certain options. I think it would have been much better to add that statement after a concerted effort has been made to reach parity. And I think this issue, the roadmap for v3.0, encapsulates that goal nicely.

[reqshark]

drop old IE support, keep it in require('xhr/legacy') or just let people use 2.x

@naugtur will IE 9 be supported or will that version be covered by xhr/legacy?

[TehShrike]

I've maintained forks of this library downstream for months while trying to get fixes merged. That's nothing against maintainers at the time, this takes time and work and we maintainers can't drop everything to push for the next milestone/merge.

It is frustrating for users, though. Part of the reason I started commenting and opening more pull requests was to try to get this library caught back up.

Leaving a bunch of changes in a long-running branch to languish unpublished is just going to make it difficult to ever be caught up.

I maintain a hundred libraries. If I had to worry about un-released changes sitting around when I went back to add a fix or feature, I'd never be able to go back to them and do necessary work when I need to months later.

[TehShrike]

I'll tray another tack: numbers 2 and 3 on this roadmap don't necessarily imply breaking API changes, and there's no reason to delay a version bump until they are resolved.

[naugtur]

@reqshark we'd drop most of the support for browsers that don't have a standard compliant XMLHttpRequest object, so in IE8 and IE9 cross-origin will no longer work. Also, tweaks for IE8 will go away. New version should be usable (with some hacks) on IE9 for very basic things.

@TehShrike @mhart I think you're both right :)
We should be able to figure out what the breaking changes will be and focus on releasing the breaking changes as 3.0 and then work on non-breaking changes for 3.1, 3.2 ...
It'd be a bad thing for our userbase if we quickly released v3, v4 and so on.

Anyway, I think we should focus on figuring out how to run cross-browser tests of the whole thing, because every new service for running browser tests I found would not let us make requests outside or start a node server. It was only possible with (now deceased) Testling. I've been testing on localhost with browsers on VMs ever since.

[dhritzkiv]

Another breaking behaviour change to add to the list:

Remove backwards compatibility of json option accepting a body-like value. Instead, have it only accept a boolean.

So

if ("json" in options && options.json !== false) {

becomes

if (options.json === true)

Related issues/PRS: #141 #142 #134 #139

[dhritzkiv]

Another: Remove some dependencies that are overkill. Namely xtend and is-function

xtend is used in one place:

//L39...
else {
    params = xtend(options, {uri: uri})
}
//...

Where this can simply be achieved with:

//L39...
else {
    params = options;
    params.uri = uri;
}
//...

And is-function, since typeof options === "function" is enough of a check, especially in the browsers that v3.0 will support.

[ianwremmel]

I'd probably do things a bit differently now, but if it's helpful, (https://github.com/ciscospark/spark-js-sdk/blob/master/packages/http-core/src/request/request.shim.js is the code I wrote to make xhr api-compatible with request.

Regarding cross browser testing, I've had reasonably good success using karma to run tests on sauce labs and talk to a local node server.

[naugtur]

I looked at the http-core. Good stuff. Far too many features for this library though - the idea behind xhr is to be a minimalistic api subset and v3 should make it even smaller in byte size for dropping IE

Could you point to a setup of tests with saucelabs? I tend to consider karma an overkill, but I'd happily use bits of the setup if you have it in public anywhere.

[ianwremmel]

yea, i agree, http-core is too much for your needs, but I thought the transforms applied to the options object in request.shim.js might help illustrate what i needed to do to make the apis agree with eachother. Obviously the promisiness is out of scope and the formdata/file-handling is debatable.

That same repo has has the karma and sauce config setup that I use. It's a little complex at this point for historical reasons. Even though they're deprecated at this point, the simplest files to look at are karma.conf.js and Gruntfile.js. Basically, I'm using karma to rerun the same set of tests that I run with mocha in node (ciscopark needs to work in both server and browser environments).

Karma has a pretty simple to use plugin for sauce labs. The only real tricky part is getting a sauce tunnel running first so that you reach the local server. Most cloud CI tools have straightforward instructions for doing so - I'm using Jenkins, so I had to write most of that from scratch.

Sidenote: I agree karma always feels like overkill. I've also found it to be the only solution that actually solves my problem :)

[jamesplease]

@naugtur , I'd be happy to help out with setting up Travis + SauceLabs. Here's a PR demonstrating it that uses Gulp, although the Gulp part is entirely optional.

Aside from what's in that PR, I:

1. made a free open source SauceLabs account
2. made a free open source Travis CI account
3. grabbed my sauce labs token, and added it to the project's Travis build as a secret env variable
4. made all of the code changes that you see in that PR

---

As an aside, I'm 👍 to the v3 roadmap. I was going to suggest dropping all of the polyfills as another option for users, but I see that's been suggested already.

[naugtur]

@jmeas I tried, but I just didn't have enough patience to get through with it. That'd be great if you could set up SauceLabs for xhr.

Invited you to be a collaborator, as per open opensource spirit.

We wanted to configure it with browserstack instead, based on a few opinions, see #81
But as I said, I tried with browserstack and didn't get it to work. I remember having SauceLabs almost set up.

[jamesplease]

Awesome. I'll work on that soon. Is there a Gitter / IRC / Slack I can reach ya at? Setting up the different pieces might be easier if we were chatting realtime.

[naugtur]

I sometimes jump into IRC, but I'm not a regular. I've started a gitter here https://gitter.im/naugtur-xhr/