[Bug] Preserve comments and styling in YAML configuration
Whenever I update Yarn 2 or I want to test a branch with yarn set version from sources it changes the yarnrc.yml.
The order of the fields changes and the commented lines disappear, is it possible to keep the file as close as possible to the state in which the user left it?
Hi! 👋
This issue looks stale, and doesn't feature the reproducible label - which implies that you didn't provide a working reproduction using Sherlock. As a result, it'll be closed in a few days unless a maintainer explicitly vouches for it or you edit your first post to include a formal reproduction (you can use the playground for that).
Note that we require Sherlock reproductions for long-lived issues (rather than standalone git repositories or similar) because we're a small team. Sherlock gives us the ability to check which bugs are still affecting the master branch at any given point, and decreases the amount of code we need to run on our own machines (thus leading to faster bug resolutions). It helps us help you! 😃
If you absolutely cannot reproduce a bug on Sherlock (for example because it's a Windows-only issue), a maintainer will have to manually add the upholded label. Thanks for helping us triaging our repository! 🌟
Status update: I've started and mostly finished a project called enhanced-yaml built on top of the yaml library which can modify YAML files while preserving comments and styling with very high accuracy. I've tested it in Yarn and it does its job quite well with .yarnrc.yml files (only a few tweaks remaining).
The biggest problem is that it's based on the yaml library, which, even though it has a powerful AST and great ways to manipulate it, is much slower than js-yaml for parsing and much slower than our custom stringifier for stringifying.
Speed comparison on a simple install on our repository:
-
parseSyml(aka how much time does it take to parse our0.9 MBlockfile):- Before (via
js-yaml):116 ms - After (via
enhanced-yaml, aka viayaml, as my package has a simple wrapper around the parser that doesn't currently do any extra work):776 ms
- Before (via
-
stringifySyml(aka how much time does it take to stringify our resolution data into the0.9 MBlockfile):- Before (via our custom stringifier):
61.7 ms - After (via
enhanced-yaml, aka viayaml, as my package doesn't do any extra work when the original source isn't passed as an argument):1374 ms
- Before (via our custom stringifier):
With the current speed stats, we can't replace the current parser and also not the current stringifier which is used for the lockfile. Because of this, currently our only option is to keep using js-yaml for the parser, our custom stringifier for the lockfile, and enhanced-yaml for the configuration (the only place where preserving comments and styling matters).
Unfortunately, using this combination would add quite a lot of extra KB to our bundle size (didn't test it with the "all 3" combination, but when replacing js-yaml and our custom stringifier with enhanced-yaml, the bundle size jumped up from 1.72 MB to 1.83 MB). The problem is that yaml is quite a heavy library because of how feature-packed it is (my wrapper library is a 4 KB wrapper in comparison - when minified) and it also can't be easily tree-shakeable because of the heavy use of classes.
Because of this, I'm curious how wanted this feature (fix?) is and if it would be worth it to implement (you can add reactions to this issue). In the meantime, I'll try to (hopefully) optimize the bundle size and see if it would be practical for us to include this.
Note: We'd also have another use for this library. We need a custom duplicate handler for some special lockfile merge conflict cases and I'm intending to implement a custom duplicate handler option inside enhanced-yaml (possible because of yaml's awesome AST, not possible with js-yaml).
Another note: I've also looked into using other YAML libraries, but none of them except yaml have what we need.
@paul-soporan I just came to search the issues after Yarn deleted my comments from .yarnrc.yml file (which made a bit angry to be honest 😄; I hate when tools silently drop my data) so it's great to see your thoughtful comment above.
I'll just say a few things:
- Reordering
.yarnrc.ymlis unpleasant. In fact, we usually run a command likeyarn set version, then copy the relevant line, revert the whole file and re-add manually. Doesn't feel right. - Data loss is plan unacceptable, IMO.
- Bundle size wouldn't be a problem if the binary wasn't committed to a Git repo – I guess we're waiting for #969 or pmm so generally yeah, you probably need to worry about bundle size and make difficult tradeoffs.
@paul-soporan
Because of this, I'm curious how wanted this feature (fix?) is and if it would be worth it to implement (you can add reactions to this issue).
Personally I don't feel a huge need, but I saw a psychological effect on teams when migrating or upgrading to yarn 3. Can't explain in details, but I realized comments are helpful for many people. Loosing comments on update is not well perceived and saw "as one more yarn bug"). It's not a big deal to handle, but if a better option exists, why not ?
Status update: I've started and mostly finished a project called enhanced-yaml built on top of the yaml library which can modify YAML
It's a very nice addition indeed. I quickly drafted a P/R to see what [email protected] would bring as an overhead. I wasn't aware you tried the same road.
The P/R is there, with some (I hope) helpful insights / findings.
https://github.com/yarnpkg/berry/pull/4135
But the your enhanced-yaml seems more advanced already. And between v1 and v2 the size increase stays valid. Haven't checked if speed is better in v2.
I'm curious how wanted this feature (fix?) is and if it would be worth it to implement (you can add reactions to this issue).
I think it would be quite helpful to get this fixed. My yarnrc.yml contains useful comments and gets overwritten :
- every time I update Yarn
- every time I set a login token for a private repository on AWS CodeArtifact (as per their docs instructions)
I understand the bundle size concerns, but this does feel like a bug. Maybe as a temporary (and admittedly ugly) band-aid, a warning could be shown that comments have been stripped?
Thank you!
@lensbart
The road i tried didn't give what i hoped for.
bundle size is okay, but too slow in my tests
https://github.com/yarnpkg/berry/pull/4135#discussion_r810542028
Another idea welcome
While I agree that the best thing would be to have comments persist in the file, I think Yarn should insert a comment at the top of the file at each edit saying something like:
# Do not add comments to this file.
# They will disappear whenever Yarn makes updates to it.