Upgrade of Undici is a breaking change

[jsumners-nr]

#85

That PR updates Undici from 5.22.1 to 6.7.0. This results in breaking on Node v16 because ReadableStream is not a global (see nodejs/undici#3049).

The upgrade of Undici should be reverted and pushed to a new major.

[JoshMock]

As per the README, our clients and their related libraries follow the Elasticsearch versioning and release schedule, so dropping support for EOL versions of Node.js will happen between minor version releases.

If you need to continue running an EOL version of Node.js, we recommend setting your @elastic/elasticsearch version in package.json using ~ (most recent patch release for your minor) rather than ^ (most recent minor release for your major).

[jsumners-nr]

If you need to continue running an EOL version of Node.js, we recommend setting your @elastic/elasticsearch version in package.json using ~ (most recent patch release for your minor) rather than ^ (most recent minor release for your major).

That isn't going to fix things at this time. This is a transitive dependency of @elastic/elasticsearch and is qualified there with a ^. Without a new release of @elastic/elasticsearch there isn't a release to pin that will not include this broken dependency.

[JoshMock]

True enough. In that case, you can use an override in your package.json:

"overrides": {
  "@elastic/elasticsearch": {
    "@elastic/transport": "~8.4.1"
  }
}

I understand that's not ideal and will see if it's reasonable to make future versions of the client depend on the latest patch rather than latest minor of @elastic/transport.

That said, Node.js v16 reached end of active support 17 months ago, and end of security support 6 months ago, so it would be a good idea to consider that upgrade as soon as is reasonable, regardless of this particular concern.

[jsumners-nr]

I understand all of that and agree with you. But it is not okay to make breaking changes in a minor version regardless of those facts.

[JoshMock]

I totally get it. We do our best to still follow semver despite being bound to ES's release schedule, but there are multiple years between major versions of ES, which would put us in an even worse position if we didn't drop Node.js versions. If we didn't drop support for older Node.js versions between minors, the client would still have to support Node v12. We'd likely have to pin to several older, less secure versions of the client's dependencies in that situation.

An alternate option, of course, would be for this library to not follow the ES release schedule, but that's a decision that would require a large effort to undo at this point. 🙂

[JoshMock]

Just as a follow-up: over in #91 I decided to publish patch versions of older 8.x versions to set a patch-only dependency (e.g. ~8.4.1 instead of the current ^8.4.1), to help ease the burden for folks on older versions of Node.js.

[jsumners-nr]

Speaking independently of anyone else, it seems to me that you're just making more work for yourself. I suppose it looks pretty if the Node.js module has the same major version number as the ElasticSearch product, but it's really not possible due to this exact situation. You should be able to publish majors when necessary.

[JoshMock]

I agree. The precedent/requirement was set across all of our official language clients, long before I took over as maintainer of this project. There are benefits to users being able to easily determine compatibility with their version of ES, but it inevitably results in problems just like this.

[kirrg001]

I understand all of that and agree with you. But it is not okay to make breaking changes in a minor version regardless of those facts.

100% agree with that.