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] Empty key in YAML sets value to null #751

Open
ddbtrmatic opened this issue Nov 17, 2023 · 2 comments
Open

[BUG] Empty key in YAML sets value to null #751

ddbtrmatic opened this issue Nov 17, 2023 · 2 comments

Comments

@ddbtrmatic
Copy link

ddbtrmatic commented Nov 17, 2023

Describe the bug

Merging two YAML config files results in a null value if the last file has an empty key.

Example:

# default.yml

db:
  global:
    host: something
    options:
      useUTC: true
# development.yml

db:
  global:
    host: something_else
    # options key left blank
    options:

Result:

{
  db: {
    global: {
      host: 'something_else',
      options: null
    }
  }
}

Expected behavior
I would intuitively expect that the empty key is treated the same as an empty object in JSON and merged.

Please tell us about your environment:

  • node-config version: 3.3.7
  • node-version: 18.18.0

Other information

@lorenwest
Copy link
Collaborator

You would expect options to be an empty object? Why not an empty string? Or a numeric zero?

I guess the answer is - what does the YAML parser treat it as? My guess is it treats it as null, which is why your result ended up with null

@ddbtrmatic
Copy link
Author

The confusion is caused by the fact that adding a value to the empty options changes the semantics of how node-config handles it, from a replacement to a merge. Someone familiar with the internals of YAML may figure this out, but a laymen could easily fall into this trap without understanding what's happening. The ambiguity of the YAML format itself leads to the confusion, so to your point, I don't expect a change.

However, it might be worth adding warning to the docs, similar to the warning that's already present about array replacement.

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

2 participants