fix: sort and quote keys with Yarn's rules when writing yarn.lock #5659
Conversation
shalvah
changed the title
|
@npm/cli-team This would be really cool to have in v9 (and 8.19.3)! |
| } | ||
|
|
||
| const shouldQuoteString = str => { | ||
| return str.indexOf('true') === 0 || |
There was a problem hiding this comment.
is there a reason this isn't a test of equality with the string true or false?
| str.indexOf('false') === 0 || | ||
| /[:\s\n\\",[\]]/g.test(str) || | ||
| /^[0-9]/g.test(str) || | ||
| !/^[a-zA-Z]/g.test(str) |
There was a problem hiding this comment.
If it starts with 0-9 then it doesn't start w/ a-zA-Z so the 0-9 check isn't needed.
| } | ||
|
|
||
| const quoteIfNeeded = val => { | ||
| if (typeof val === 'boolean' || typeof val === 'number' || shouldQuoteString(val)) { |
There was a problem hiding this comment.
idk if we need two functions here. single use shouldQuoteString logic can be inlined w/ the rest of this if statement.
|
Tweaked and consolidated things in #5724 in the interest of getting this out w/ today's release. |
Yes please @shalvah anyone who is willing to own this is more than welcome to collaborate w/ us on keeping npm up to spec. |
|
Alright @wraithgar . Thanks for picking it up. Guess I'll close this PR then. Currently in the hospital with a broken arm so I couldn't make any changes. 😓Will hopefully work on the future improvements when I'm better. |
|
Hope you get better soon. Looking forward to future PRs. |
3 participants
This PR adjusts the logic used for sorting keys when writing
yarn.lockto be more in line with Yarn's own rules, avoiding unnecessary diffs and confusion.References
Addresses some of the issues raised in #5126, by mostly adapting the code Yarn itself uses. It doesn't solve everything (I didn't address the registry URLs issue, and sorting differs slightly when there are
~or^), but it aligns this more closely with what Yarn produces. If the npm team is interested, I can work on PRs to try to address the others.