commentBefore property has no effect

[rulatir]

test.js:

const YAML = require('yaml');

const input =
"version: 1\n\
task_definition:\n\
  services:\n\
    some_service:\n\
      secrets:\n\
        - name: SOME_NAME\n\
          value_from: SOMEWHERE\n";

const parsed = YAML.parse(input);
const generatedSecretBindings = YAML.createNode([
    {
        name: "ANOTHER_NAME",
        value_from: "ANOTHER_SOURCE"
    }
])
generatedSecretBindings.commentBefore =
    "Don't edit these secret bindings directly. They are generated.";
parsed.task_definition.services.some_service.secrets = generatedSecretBindings;
console.log(YAML.stringify(parsed));

Actual output:

version: 1
task_definition:
  services:
    some_service:
      secrets:
        - name: ANOTHER_NAME
          value_from: ANOTHER_SOURCE

Expected output (more or less):

version: 1
task_definition:
  services:
    some_service:
      # Don't edit these secret bindings directly. They are generated.
      secrets:
        - name: ANOTHER_NAME
          value_from: ANOTHER_SOURCE

Additional information:
node --version: v12.11.1

    "yaml": {
      "version": "1.7.1",
      "resolved": "https://registry.npmjs.org/yaml/-/yaml-1.7.1.tgz",
      "integrity": "sha512-sR0mJ2C3LVBgMST+0zrrrsKSijbw64bfHmTt4nEXLZTZFyIAuUVARqA/LO5kaZav2OVQMaZ+5WRrZv7QjxG3uQ==",
      "requires": {
        "@babel/runtime": "^7.5.5"
      }
    }

[eemeli]

That usage pattern should work, but it clearly doesn't.

What's happening here is that YAML.stringify() is first creating a YAML document from its input, and then stringifying that. During the stringification, the parsed object is recursively worked into a tree of YAML nodes, and that process does not (currently) provide special handling for YAML nodes that already exist in the tree. But it probably should. I'll need to test a few things and work out the cleanest way of making that happen.

As is, here's how you could do what you're trying to do with the current API, with fewer layers of conversion taking place internally:

const doc = YAML.parseDocument(input)
const secrets = doc.getIn(['task_definition', 'services', 'some_service', 'secrets'])
secrets.commentBefore = " Don't edit these secret bindings directly. They are generated."
secrets.set(0, { name: 'ANOTHER_NAME', value_from: 'ANOTHER_SOURCE' })
String(doc)

version: 1
task_definition:
  services:
    some_service:
      secrets:
        # Don't edit these secret bindings directly. They are generated.
        - name: ANOTHER_NAME
          value_from: ANOTHER_SOURCE

[rulatir]

Actually I want to completely replace the secrets node, so perhaps I could just createNode from an array and then:

const someService = doc.getIn(['task_definition','services','some_service']);
someService.set('secrets',secrets);

Will that work?

Anyways, the lack of special handling for YAML nodes already in the tree is precisely what I suspected.

[rulatir]

Actually if there is no special handling for YAML nodes, then why doesn't output look like this:

version: 1
task_definition:
  services:
    some_service:
      secrets:
        items:
          - key: name
            value: ANOTHER_NAME
          - key: value_from
            value: ANOTHER_SOURCE
        commentBefore: "Don't edit these secret bindings directly. They are generated."

?

[eemeli]

Yeah, that should work. There's also this API (these are documented here):

doc.setIn(['task_definition','services','some_service','secrets'], secrets);

The fix for your original issue looks like it'll be a one-liner. The reason why you're not seeing a commentBefore node in the output is that when stringifying unknown objects we first look for a toJSON() method and call that if it's available. For YAML nodes, that's currently transforming them back into plain JS objects before they're re-parsed into nodes.

[eemeli]

Fixed in 1.7.2.