Skip to content

Drop support for jsdom 9.x. #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 6, 2020
Merged

Drop support for jsdom 9.x. #43

merged 2 commits into from
May 6, 2020

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented May 5, 2020

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.

sjrd added 2 commits May 5, 2020 12:15
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.
@sjrd sjrd requested a review from gzm0 May 5, 2020 10:34
@gzm0
Copy link
Contributor

gzm0 commented May 5, 2020

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
Copy link
Member Author

sjrd commented May 5, 2020

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
Copy link
Contributor

gzm0 commented May 6, 2020

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.

@gzm0 gzm0 merged commit 9dde29e into scala-js:master May 6, 2020
@sjrd sjrd deleted the drop-jsdom-9 branch May 6, 2020 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants