search

scala-js / scala-js-env-jsdom-nodejs

Node.js with jsdom environment for Scala.js
BSD 3-Clause "New" or "Revised" License
8 stars 10 forks source link

Drop support for jsdom 9.x. #43

Closed sjrd closed 4 years ago

sjrd commented 4 years ago

jsdom 10.0.0 is 3 years old. We held on to jsdom 9.x until now because we wanted an upgrade path from Scala.js 0.6.x, which has historically supported jsdom 9.x. Now it is time to move on.

Best reviewed with the "Hide whitespace changes" option enabled.

gzm0 commented 4 years ago

Changes LG. I feel like we should discuss if this needs a major version bump. It clearly removes functionality that was there. (And it's cheap for us to actually do this here).

sjrd commented 4 years ago

My opinion would be that a major version bump should be reserved for when we break the JVM binary API. We should be able to stop supporting an old version of a dependency in a minor version, I think, since that doesn't create a schism in the ecosystem.

If we only had JVM dependencies, we allow ourselves to bump our libraryDependencies to a newer version of that dependency. That effectively drops support for using older versions of that library in a user's classpath. So clearly we allow that in minor version bumps for JVM dependencies.

By that reasoning, a minor version bump can drop support for a JS dependency as well.

WDYT?

gzm0 commented 4 years ago

Hmmm... Interesting. Of course the SemVer spec doesn't actually say something about dependencies :)

At first you almost had me convinced. But thinking about it a bit more, it feels like not all dependencies are created equal: I'd distinguish them by whether they show up in the API.

Updating an internal dependency should be a minor version (maybe even patch): It doesn't change the functionality of the package, it just changes parts of the implementation (which happens to be provided by a library). This is even if we run the risk of trapping our users in classpath dependency hell.

Updating an external dependency however needs more consideration: For example, if we upgrade the dependency of this repo on scalajs-js-envs to a hypothetical 2.x we (implicitly) break the public API.

At this point, it boils down to deciding whether JSDom is an internal or an external dependency of this repo. The more I think about it, there is no part of the publicly visible surface that uses the JSDom API. The sandbox itself is driven by the DOM specification.

Long story short: I agree, this should be a minor version bump only.