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

[Bug?] undefined is converted to null #173

Closed
marvin-j97 opened this issue May 18, 2020 · 5 comments
Closed

[Bug?] undefined is converted to null #173

marvin-j97 opened this issue May 18, 2020 · 5 comments
Milestone

Comments

@marvin-j97
Copy link

marvin-j97 commented May 18, 2020

Version

Using 1.10.0

Reproduce

const YAML = require("yaml");

const obj = () => ({
  a: 4,
  b: "str",
  c: null,
  d: undefined,
});

const jsonified = JSON.parse(JSON.stringify(obj()));
console.log(jsonified.hasOwnProperty("d")); // -> false
console.log(jsonified["d"]); // -> undefined

const yamlified = YAML.parse(YAML.stringify(obj()));
console.log(yamlified.hasOwnProperty("d")); // -> true???
console.log(yamlified["d"]); // -> null

I don't see how this would be intentional. undefined is not equal to null.

@eemeli eemeli added this to the yaml 2 milestone May 21, 2020
@eemeli
Copy link
Owner

eemeli commented May 21, 2020

This was a design choice of mapping undefined to null in YAML, as it's the closest match to an empty value. But I do see your point that this is different from what JSON.stringify() does.

I think this should be considered in terms of the 2.0 update, as changing the API like this can be a breaking change.

@kyeotic
Copy link

kyeotic commented Aug 10, 2020

I would like to vote that undefined keys not remain in the stringified version. I am now manually preprocessing to remove these keys, deeply.

@eemeli
Copy link
Owner

eemeli commented Aug 15, 2020

Yeah, happy to include this fix in the major update. A PR on this would be welcome; I'm finding myself spending a bit more time on a couple of other projects atm.

@kyeotic
Copy link

kyeotic commented Aug 18, 2020

@eemeli I tried to dig into this. The code is pretty complicated, but it looks like some information is lost when construction the node graph so that undefined is converted into null as the Pair is made. That means the change can't be isolated to stringify, it needs to be part of the document's construction.

@eemeli
Copy link
Owner

eemeli commented Aug 23, 2020

@kyeotic The right place to fix this was in tags/failsafe/map.js, where the JS Object's entries are added to the new mapping. By the time we're creating the Pair it's already too late.

JSON.stringify([1, undefined, 2]) === '[1,null,2]', so it makes sense to also have this behaviour be object-specific in yaml.

I also opted to keep undefined values in Map objects as such, because I figured that's an intentional choice from the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants