Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Put ls-engines in library mode #215

Closed
wants to merge 4 commits into from
Closed

Put ls-engines in library mode #215

wants to merge 4 commits into from

Conversation

lishaduck
Copy link
Contributor

Split off from: #190

@lishaduck
Copy link
Contributor Author

lishaduck commented Jul 9, 2024 •
edited
Loading

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

@lishaduck
Copy link
Contributor Author

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

@jfmengels
Copy link
Owner

🤦‍♂️ 😮‍💨

@lishaduck
Copy link
Contributor Author

lishaduck commented Jul 20, 2024 •
edited
Loading

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

lishaduck commented Sep 3, 2024 •
edited
Loading

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

@socket-security Socket Security
Copy link

socket-security bot commented Sep 4, 2024 •
edited
Loading

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
chore: update snaopshots
43a9ed7
@lishaduck
chore(deps): bump elm deps
804d0d9
@lishaduck
ci: ls-engines in library mode
d185590
@lishaduck
chore: fix ls-engines
b38cf28 
Ok, so this one is complicated.
SemVer wise, we should be able to support glob 10 & glob 11.
The only breaking change is the engine.
Therefore, the "correct" specifier is indeed `^10.2.6 || ^11.0.0`.
We support both, if you need an older engine,
you should use an older version.
In fact, npm is supposed to do this automatically.
However, up until `npm-pick-manifest@9.1.0` (included in `npm@10.8.2`),
there was a longstanding bug where it didn't!
The code was there,
but the logic didn't work right and after Isaacs moved on to vlt,
his PR fixing this got stalled.
After the `glob` fiasco, he rebased the PR and it's now fixed,
but the change wasn't backported.
Because we support Node 14, we support `npm@6.14.18`,
so we can't take advantage of this.
Instead, we just have to tolerate a slightly old version of glob.
Oh well.

I also removed "enginesStrict" because our engine declaration is now higher than technically needed
because glob only declares support for the latest versions of Node branches.
@lishaduck
Copy link
Contributor Author

lishaduck commented Oct 23, 2024 •
edited
Loading

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.

@lishaduck lishaduck mentioned this pull request Oct 29, 2024
Merged
@jfmengels
Copy link
Owner

Superseded by #274

@jfmengels jfmengels closed this Oct 29, 2024
@lishaduck lishaduck deleted the fix-ls-engines branch November 4, 2024 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lishaduck @jfmengels