Revert "CI: Work around a weird bug in Yarn v1.x" #131
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 12c276b. Revert a now-unnecessary workaround. Amazingly, this issue has been avoided in the latest npm versions, as npm have restored the files Yarn 1.x rather over-optimistically required npm to have. NodeJS has in turn bundled these versions of npm with the fixes before the final minor/patch release of every major version of NodeJS. This is effectively a solved issue if you use the latest patch versions of NodeJS. No more incompatibility with Yarn 1.x in these narrow cases. (Admittedly, the Yarn 1.x issue was due to a genuine bug in Yarn 1.x, not a deliberate thing from the Yarn authors I don't think).
confused-Techie
approved these changes
Jul 23, 2024
|
Appreciate the review, merging! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change, Rationale and Context
Change
This reverts commit 12c276b.
Background/Context
Revert a now-unnecessary workaround.
Amazingly, this issue has been avoided in the latest npm versions, as npm have restored the files Yarn 1.x rather over-optimistically required npm to have.
NodeJS has in turn bundled these versions of npm with the fixes before the final minor/patch release of every major version of NodeJS.
This is effectively a solved issue if you use the latest patch versions of NodeJS. No more incompatibility with Yarn 1.x in these narrow cases.
(Admittedly, the Yarn 1.x issue was due to a genuine bug in Yarn 1.x, not a deliberate thing from the Yarn authors I don't think).
Research Notes
See this comment for all the (somewhat exhaustive!) research I did to support making this change: npm/cli#6554 (comment)
Side note: Reverting the workaround is possible now that the Yarn 1.x issue has been effectively (and rather graciously) resolved by npm and NodeJS, even though it's to keep compatibility with a rather strange decision Yarn 1.x's authors made (+ an inadvertent bug in Yarn 1.x's code, it would seem) several years ago now in a now-essentially-frozen codebase. Thank you npm team for making the best move for the ecosystem, even if it means leaving a bit of cruft around in your own flagship project's repository!