feat: drop Node 10 support

[MichaelDeBoey]

• If we want to update our @testing-library/dom dependency to v8, we also need Node@>=12
  https://github.com/testing-library/dom-testing-library/blob/1fc17bec5d28e5b58fcdd325d6d2caaff02dfb47/package.json#L23-L25

• If we want to update our @typescript-eslint/parser dependency to v5, we also need Node@^12.22.0 || ^14.17.0 || >=16.0.0
  https://github.com/typescript-eslint/typescript-eslint/blob/b9407c560c8ab625fd546af73f71cce8178b9e05/package.json#L55-L57

• Node 10 is EOL, so it's a good practice to not support it anymore.

• ESLint 8.x requires Node@^12.22.0 || ^14.17.0 || >=16.0.0 and imo it's a good practice to keep the Node version equal to that of the latest supported ESLint version
  https://github.com/eslint/eslint/blob/fa4d4830a0e77f92154079ada17ffb893ce64232/package.json#L146-L148

---

BREAKING CHANGE: Requires Node@^12.22.0 || ^14.17.0 || >=16.0.0

As requested by @G-Rath in #200 (comment)

This branch is dependent on #237

[codecov]

Codecov Report

Merging #238 (4bc77bd) into main (3f59068) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main      #238   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          561       561           
  Branches       157       157           
=========================================
  Hits           561       561           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f59068...4bc77bd. Read the comment docs.

[G-Rath]

Since this is a breaking change, I think we should bundle it together with other breaking changes (e.g. upgrading to @testing-library/dom v8 - though I wonder if we can support both since it doesn't seem like we need to do any code changes to support the new version?).

(alternatively merging them into a next branch would work too, but I don't mind a single PR if there's not a lot of changes).

[MichaelDeBoey]

@G-Rath Once we drop Node 10, it won't be a breaking change anymore to upgrade @testing-library/dom

[G-Rath]

@MichaelDeBoey sounds good, though I think we should actually make @testing-library/dom a peer dependency: it's being used only to get a list of the query methods, which means we shouldn't be having our own version of it on the tree incase our version has different query methods than other versions that are being used.

That'd let us support both versions without a new major, and be more robust.

(Again I'm not against doing a breaking version, just think the more changes we can land before we drop this version of node the better, and this should be an easy one)

[MichaelDeBoey]

@G-Rath Moving @testing-library/dom to peerDependencies would be another breaking change though.

Apart from that, I'm not sure if we should move it tbh, as this ESLint plugin is to check for errors when using @testing-library/jest-dom.
This means that we can assume @testing-library/jest-dom is present, but it doesn't automatically mean @testing-library/dom would be present.

If that's not the case, we force people of this plugin to include it just to make our plugin happy.
Therefore I think it's better to keep it in dependencies list instead.

CC/ @benmonro & @nickmccurdy as they are the original creators/maintainers of this plugin.
Also CC/ @kentcdodds as he probably has some valuable input in this as wel.

[G-Rath]

@MichaelDeBoey yeah actually that is still a breaking change for anyone not on npm 7+.

it doesn't automatically mean @testing-library/dom would be present.

No but we do require it & want to be using the same version as what's being used in the project for the reason I stated above - which is exactly what peer dependencies are for and why they were always meant to be installed by default like standard dependencies (which they now are in npm 7+).

The hassle you've described applies in both situations: by not having it as a peer dependency here, this package can cause package managers that do install peer dependencies by default to use whatever version is specified by this package which currently breaks builds by being an outdated version (as described here) - the result is that we have to specify @testing-library/dom as a dependency to force it to use a higher version (which also results in the duplication, since this package requires a different version meaning code is now being linted with a different version of the package than what it's run with).

[G-Rath]

Therefore I think it's better to keep it in dependencies list instead.

While I understand there is not a lot of difference in what I've just described, consider it from the UX POV:

If specified as a dependency:

Developer could get any sort of behaviour or error depending on what the actual problem is, none of which point you in the direction of "you need to explicitly install this package at a higher version because ...".

(the error I got was one about timers not working, due to jest 27 changing it's timer mock implementation)

If specified as a peer dependency:

Developer will always get an error about not being able to require/import @testing-library/dom, for which most developers knee-jerk reaction is <package manager add command> -D @testing-library/dom; we could even wrap it in a try/catch to throw a message saying "hey you need to have this as a dependency" if we wanted to be even more helpful.

[MichaelDeBoey]

@G-Rath I understand your reasoning but still not 100% convinced for peerDependencies listing tbh.

Can I maybe contact you on Twitter/Discord or so to have a little chat about this (and also discuss a possible solution to not have 2 breaking changes released after each other)?

[G-Rath]

@MichaelDeBoey I'm not saying we need to do it anytime soon, just pointing out that I think in the long-run technically we should - it'd matter a lot more if we expected queries to be changed a lot, or if more package managers (aka yarn) installed peer dependencies by default but since that's not the case and it'd be a breaking change either way we might as well just drop the node version for now 🤷

I'm not on Twitter, but my discord is G-Rath#1970

[MichaelDeBoey]

Let's create a separate issue/discussion about adding @testing-library/dom to peerDependencies and merge this one already.
This way @benmonro, @nickmccurdy & @kentcdodds (and other people) can have their say about it.

If the issue/discussion comes to a conclusion, we can always decide to create another breaking change if necessary.

[github-actions]

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀