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

Object wrappers for primitives may not be stringified correctly #236

Closed
MikeRalphson opened this issue Mar 4, 2021 · 3 comments
Closed
Labels
bug Something isn't working

Comments

@MikeRalphson
Copy link
Contributor

MikeRalphson commented Mar 4, 2021

Describe the bug
JavaScript types Boolean, Number and String (wrappers for primitive types) are stringified very differently to in JSON when invoked with new.

To Reproduce

const yaml = require('yaml');

const o = { enum: [] };
o.enum.push(new String('mystring'));
o.bool = new Boolean(false);
o.num = new Number(123);

console.log(JSON.stringify(o));

console.log(yaml.stringify(o));

Expected behaviour

enum:
  - mystring
bool: false
num: 123

not

enum:
  - - m
    - y
    - s
    - t
    - r
    - i
    - n
    - g
bool: {}
num: {}

Versions (please complete the following information):

  • Environment: Node v14.15.1
  • yaml: 2.0.0-3

Additional context

JSON.stringify returns {"enum":["mystring"],"bool":false,"num":123}

Unsure if this is really a bug in the default operation, or whether the behaviour could be controlled via an option. Does not happen when the new keyword is not used.

@MikeRalphson MikeRalphson added the bug Something isn't working label Mar 4, 2021
@eemeli
Copy link
Owner

eemeli commented Mar 4, 2021

That's a difference from the behaviour of JSON.stringify, so it definitely counts as a bug. I'll need to investigate a bit if those are the only cases that are treated specially, or what the exact logic might be.

I don't really see that it'd be beneficial to enable the current incorrect and surprising behaviour with an option.

@eemeli
Copy link
Owner

eemeli commented Mar 4, 2021

Yeah, they're effectively special-cased by the spec. In addition to these specific three, there's also the object form of BigInt that's unwrapped to a primitive when stringifying.

I for one had no idea that even existed before now. I mean, if you try new BigInt(42), it'll throw a TypeError. But you can actually construct it with Object(42n), though I can't imagine why you'd want to do that*. With JSON.stringify that'll then throw at a later stage with Do not know how to serialize a BigInt, but we can handle it and then be fine re-parsing it with intAsBigInt: true.

* This is a lie. I actually have a potential use case for exactly this, but I'm trying very hard not to think about it too much, because it'd mean rewriting also the front-end of this library. This bug report has actually provided me with a core missing piece of that idea.

@eemeli eemeli closed this as completed in 7a9a8a4 Mar 8, 2021
@eemeli
Copy link
Owner

eemeli commented Mar 8, 2021

Thank you @MikeRalphson, this should be fixed now. I'm very happy for this to have come up during the v2 transition, as it means that I don't need to figure out if this counts as a breaking change or not.

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