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

"null" key parses as the empty string #554

Closed
cdaringe opened this issue Jun 8, 2024 · 5 comments · Fixed by #581
Closed

"null" key parses as the empty string #554

cdaringe opened this issue Jun 8, 2024 · 5 comments · Fixed by #581

Comments

@cdaringe
Copy link

cdaringe commented Jun 8, 2024

Describe the bug

null as a key is parsed as the empty string:

> yaml.parse('null: 1')
{ '': 1 }

To Reproduce

> yaml.parse('null: 1')
{ '': 1 }

Expected behaviour

> yaml.parse('null: 1')
{ 'null': 1 }

Versions (please complete the following information):

  • Environment: node 20.x
  • yaml: "2.4.3"

Additional context

@cdaringe cdaringe added the bug Something isn't working label Jun 8, 2024
@eemeli
Copy link
Owner

eemeli commented Jun 8, 2024

null as a key is parsed as a !!null scalar value.

The issue that you're encountering is that the JS Object only supports string keys, and the !!null value is stringifed as an empty string '' rather than 'null', as you're expecting.

If you're working with data that may include non-string keys, you could try one of the following approaches:

  1. Parse mappings as Maps:

    import { parse } from 'yaml'
    parse('null: 1', { mapAsMap: true })
    // Map(1) { null => 1 }
  2. Parse the input using the failsafe schema, which parses all scalars as strings:

    import { parse } from 'yaml'
    parse('null: 1', { schema: 'failsafe' })
    // { null: '1' }

@eemeli eemeli removed the bug Something isn't working label Jun 8, 2024
@cdaringe
Copy link
Author

cdaringe commented Jun 9, 2024

Thanks for the thoughtful response. I acknowledge that the YAML spec nods to non-string native types being supported on parse. I’m curious from your perspective, as the library author, why treating keys as strings isn’t the default behavior, given that traditionally POJOs are our mapping type in JS. In other words, I would’ve guessed that non-string scalar keys would be behind a flag, and string keys would be the default. Maybe as I research the fail safe flag I’ll learn more :-)

@eemeli
Copy link
Owner

eemeli commented Jun 9, 2024

I’m curious from your perspective, as the library author, why treating keys as strings isn’t the default behavior, given that traditionally POJOs are our mapping type in JS.

Because this is a YAML library, and YAML explicitly allows and supports non-string keys.

@cdaringe
Copy link
Author

cdaringe commented Jun 9, 2024

explicitly allows and supports non-string keys

sure, but just because yaml supports it doesn’t mean it must be processed. Some languages dont even have null. I presume they get parsed as “null”.

Is it incorrect to parse as strings? A casual scan of the spec suggests that it is not a problem.

Thus, why wouldn’t we parse using JS friendly behavior first? What use cases in JS would someone want non strings parsed into a JS object key?

@eemeli
Copy link
Owner

eemeli commented Jun 9, 2024

Is it incorrect to parse as strings? A casual scan of the spec suggests that it is not a problem.

The default schema used by this library is the YAML 1.2 core schema, which defines quite exactly how plain scalars are to be parsed, independently of where in the document structure they show up.

Thus, why wouldn’t we parse using JS friendly behavior first? What use cases in JS would someone want non strings parsed into a JS object key?

Because this library is seeking to provide an accurate representation of a YAML document, which may contain mappings that have non-string keys. Please also note that you're effectively asking for a breaking change, which I would prefer not to do unless there's a really strong case for such a change.

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

Successfully merging a pull request may close this issue.

2 participants