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.

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

Empty map values are round-tripped to null #366

Closed
GeorchW opened this issue Mar 9, 2022 · 6 comments
Closed

Empty map values are round-tripped to null #366

GeorchW opened this issue Mar 9, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@GeorchW
Copy link

GeorchW commented Mar 9, 2022

Description: Empty map values are not round-tripped correctly.

To reproduce:

parseDocument("nothing:").toString()
// expected output: "nothing:"
// actual output:   "nothing: null\n"

Versions:

  • Environment: Node.js v16.13.1
  • yaml: 2.0.0-10

Additional context

Thanks a lot for this awesome library! I'm using it to edit around in docker-compose files, where the pattern

volumes:
  a-volume:
  another-volume:

is quite common. These are currently round-tripped to

volumes:
  a-volume: null
  another-volume: null

If there is more content after the volumes block, it's getting worse, where this

volumes:
  a-volume:

networks: "whatever"

is round-tripped to this:

volumes:
  a-volume:

    null
networks: "whatever"

There are other minor offenders to round-tripping (like a tiny bit of whitespace preservation here and there), but this is the only offender that is very visible. I can probably refer to the CST for my use-cases, but this might be something you want to look at.

@GeorchW GeorchW added the bug Something isn't working label Mar 9, 2022
@GeorchW
Copy link
Author

GeorchW commented Mar 10, 2022

I've debugged a little and I believe I found the culprit:

stringify: ({ source }, ctx) =>
source && nullTag.test.test(source) ? source : ctx.options.nullStr

When the null tag is stringified, it checks if the source contains something that would be parsed as null before writing it out. I'm not sure if that's neccessary, I don't know what scenarios there are where the source attribute is not containing a string that parses to null, that's where I'm not deep enough in the code.

@eemeli eemeli closed this as completed in a3dce83 Mar 11, 2022
@GeorchW
Copy link
Author

GeorchW commented Mar 11, 2022

Wow, that was fast -- thanks a lot!

@eemeli
Copy link
Owner

eemeli commented Mar 11, 2022

You're right, that shouldn't have been happening. The test in stringify was wrong, as it didn't recognise source === '' as a valid representation of a null value.

@GeorchW
Copy link
Author

GeorchW commented Mar 11, 2022

How is the release schedule?

@eemeli
Copy link
Owner

eemeli commented Mar 11, 2022

There's no schedule as such. I'll try and roll out a new release within a week or two; there are a few other fixes I'd like to include.

@GeorchW
Copy link
Author

GeorchW commented Mar 11, 2022

Okay, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants