jfmengels / node-elm-review

CLI for elm-review
https://package.elm-lang.org/packages/jfmengels/elm-review/latest/
BSD 3-Clause "New" or "Revised" License
48 stars 25 forks source link

Put `ls-engines` in library mode #215

Closed lishaduck closed 3 weeks ago

lishaduck commented 4 months ago

Split off from: #190

lishaduck commented 4 months ago

That this fails is good, it's catching that we now need #190.

lishaduck commented 4 months ago

Great, more fun! Something else must've dropped node 14- in the whole glob fiasco. Welp. Whadawedonow?

jfmengels commented 4 months ago

:man_facepalming: :face_exhaling:

lishaduck commented 4 months ago

Oh, just saw this! npm/npm-pick-manifest#33 fixes a longstanding bug in npm, so engines-based resolution is a thing, just like it should be. Therefore, we can revert all of these engines changes, and npm won't install the newer glob/jackspeak/etc. However, if we support node 14, then we support npm 6 (and 8 and 10), so... no 😢 The next time this happens though, it should be much less of an issue 🎉

That change would change the behavior to need an ls-engines engines though, so we'll probably have to wait a week or two (but, of course, we won't drop npm <10.8.2 anytime soon, so 🤷‍♂️)

lishaduck commented 2 months ago

I've fixed this locally (by reverting f0894b30a252e7db739546efa17f2ded174d167a from #190), but I need to split the giant branch into reviewable PRs.

socket-security[bot] commented 2 months ago

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/ls-engines@0.9.3 Transitive: environment, eval, filesystem, network, shell, unsafe +264 14.8 MB ljharb
npm/strip-ansi@6.0.1 None 0 4.03 kB sindresorhus
npm/terminal-link@2.1.1 None +3 149 kB sindresorhus
npm/turbo@2.0.5 None +6 128 MB turbobot
npm/typescript@5.5.2 None 0 21.9 MB typescript-bot
npm/uglify-js@3.18.0 environment, eval, filesystem 0 1.3 MB alexlamsl

🚮 Removed packages: npm/ls-engines@0.9.2)

View full report↗︎

lishaduck commented 1 month ago

Note: I did fix this, but I don't want to push it till #256 gets merged b/c my branches are complicated enough as-is.

jfmengels commented 3 weeks ago

Superseded by https://github.com/jfmengels/node-elm-review/pull/274