pre-commit.ci compatiblity (node 20.17)

[nijel]

The latest release fails because pre-commit.ci service has node 20.17 and nano-spawn requires 20.18 (see https://results.pre-commit.ci/run/github/883563065/1747730165.8GYxDPsHSyetBPvQFawKgg).

They have released https://github.com/sindresorhus/nano-spawn/releases/tag/v1.0.2 to address this. Can you please release markdownlint-cli with updated dependency?

[DavidAnson]

Why isn't pre-commit running the latest version of Node 20? At least one of the more recent versions than 20.17 is a security release, so pre-commit are running third-party code on a known-vulnerable system in the current configuration.

https://nodejs.org/en/blog/release/v20.19.2

[DavidAnson]

Wait. nano-spawn is in this project's devDependencies, not its dependencies. Therefore, it is not required to run markdownlint-cli and pre-commit should not be installing it or complaining about it.

[nijel]

Why isn't pre-commit running the latest version of Node 20?

I have no clue, opened pre-commit-ci/runner-image#297 to figure out.

Wait. nano-spawn is in this project's devDependencies, not its dependencies. Therefore, it is not required to run markdownlint-cli and pre-commit should not be installing it or complaining about it.

It is needed because it installs and builds from git repo, see pre-commit/pre-commit#2483.

[DavidAnson]

I agree with the author of the second issue - installing dev dependencies is the wrong thing to do and projects should at least have a way to opt out of that behavior in their YAML. Note that dev dependencies could be platform-specific OR extremely large and slow to install - neither of these factors should impact end users. I bet pre-commit is paying way (way!) more than double the time and bandwidth to install dev dependencies for every project when most don't need that. (Selfishly and for example, this issue wouldn't have even come up.)

[nijel]

My knowledge of the Node ecosystem is very limited, so I really can't judge this, and I'm definitely not in position to propose some solution to pre-commit. To me, it more seems like an issue in the way dependencies are specified, there should be separate build dependencies from devel ones, but there seems no way to specify that in package.json. So to make work packages which have build dependencies (such as typescript) all devel dependencies need to be installed.

Anyway, if you would bump nano-spawn in the lock file and release a new version, it would work in the existing ecosystem.

[DavidAnson]

To be fair, markdownlint-cli already (appears to) work in the current ecosystem. What we are discussing here is accommodating incorrect behaviors (plural) by a specific environment. I acknowledge your point that this is a relatively small change, but it's 100% overhead for me and time I could spend doing something meaningful instead of working around bad tooling decisions.

[DavidAnson]

I'll try to push a patch release tonight, though I feel this should not be necessary.

[DavidAnson]

Sorry, I didn't manage to patch, BUT:

• I see you have a PR to their repo to update the Node version, thanks!
  • Update Node to 20.19.2 pre-commit-ci/runner-image#298
• The desired update commit is in this repo and you will get it if you update to pull from "master" branch instead of the tag.
  • 2621210

Either should fix this for you.

[ModeSevenIndustrialSolutions]

Can confirm that commenting "pre-commit.ci run" to re-run linting on the failed pull requests is now allowing us to merge these changes; thanks for your prompt response working on this issue 👍

[DavidAnson]

pre-commit-ci/runner-image#298 was merged and independently verified.