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

Document setIn() should work if document contents are empty #174

Closed
akprgm opened this issue May 21, 2020 · 5 comments
Closed

Document setIn() should work if document contents are empty #174

akprgm opened this issue May 21, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@akprgm
Copy link

akprgm commented May 21, 2020

my yaml file may or may not have keys and values, if doesn't have keys then i want create it. I think this issue documented as well but is there any way to achieve this.
conf.yml

external_servivces:
   service_1: 
      property_1: 1
const fs = require('fs');
const yaml = require('yaml');
const file = fs.readFileSync('./conf.yaml', 'utf8')
const doc =yaml.parseDocument(file);
doc.setIn(['external_servivces', 'service_1', 'property_1'], 1);

@eemeli
Copy link
Owner

eemeli commented May 21, 2020

I'm not sure that I understand the question here. The example above should just work. Perhaps if you could give an example which fails to work as intended?

@akprgm
Copy link
Author

akprgm commented May 21, 2020

Above example doesn't work, if the those properties doesn't exist in yaml file.

Error: Expected YAML collection at external_services. Remaining path: service_1

In my case yaml is updated by multiple teams, if there is already a key for external_service then i want to add other sub properties, else i want to create it and then add it.

Also i tried doing this, but getting the same error

if (!doc.has('external_services')) {
    doc.setIn(['external_services'], "");
    doc.setIn(['external_services','service_1'], '')
    doc.setIn(['external_services','service_1', 'property_1'], 'testing')
}

@eemeli
Copy link
Owner

eemeli commented May 21, 2020

Ah, I think I see now. When you're parsing empty input, it's not possible to determine what shape those contents are expected to have. A YAML document is not limited to having a key-value mapping at its root.

So what you should probably do is that once you've parsed the config file into a YAML document, check that it's a map, and if it's not, assign a map to the contents. After that, the setIn() will work as you expect:

import YAML from 'yaml'
import { YAMLMap } from 'yaml/types'

const doc = YAML.parseDocument(src)
if (doc.contents instanceof YAMLMap) {
  // no need to do anything
} else if (doc.contents) {
  // the contents have been parsed as something other than a mapping
  // this is probably an error
} else {
  doc.contents = YAML.createNode({})
}

doc.setIn(...)

For the particular case where contents are empty, it would make sense for the root mapping to be auto-generated because there's no pre-existing value to override. Especially as that's what setIn() already does within collections.

@eemeli eemeli added the enhancement New feature or request label May 21, 2020
@eemeli eemeli changed the title not able to add or set nested values in yaml Document setIn() should work if document contents are empty May 21, 2020
@akprgm
Copy link
Author

akprgm commented May 22, 2020

Also it's not necessary that this YAML file will be empty and not sure how above snippet solve my problem.

eg. Existing YAML Doc

other_things: abc
external_servivces: #may exist
   service_1:  #may exist
      property_1: 1 #may exist

So my end goal here is to update the value of property_1 but to set this property i will have to check if parents properties are there. This can be easily achieved using Parse and Stringify but i also want to preserve comments and line breaks here.

@eemeli
Copy link
Owner

eemeli commented Aug 23, 2020

This should now be fixed. There were in fact two separate bugs here:

  1. When parsing collections the schema wasn't being properly passed to its constructor, so the intermediate collection creation didn't work like it should have.
  2. The empty-document case now gets special handling to allow for set() and setIn() to work.

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

No branches or pull requests

2 participants