Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update addPairToJSMap.ts, missing ? operator #458

Closed
wants to merge 1 commit into from

Conversation

MikeRalphson
Copy link
Contributor

Found while testing alias preservation, using the visit method:

function parseWithAliases(str) {
  const aliases = new Map();
  const ast = yaml.parseDocument(str);
  yaml.visit(ast,function(key,node,path) {
    if (yaml.isAlias(node)) {
      aliases.set(node.source,node.resolve(ast).toJS());
    }
  });
  return { data: ast.toJS(), aliases };
}

Found while testing alias preservation, using the Visit method
@eemeli
Copy link
Owner

eemeli commented Apr 4, 2023

Would you have some example that fails without this? At least internally addPairToJSMap() should never get called with a broken ctx value.

@MikeRalphson
Copy link
Contributor Author

hello: &a
  world:
    message: Hello, world
outputs:
  *a

@eemeli
Copy link
Owner

eemeli commented Apr 4, 2023

Ah, I see now what's happening here. The error is misleading, and should be thrown earlier -- the argument is required for a node's .toJS(doc) call.

Here's a fix for your function that should make it work:

     if (yaml.isAlias(node)) {
-      aliases.set(node.source,node.resolve(ast).toJS());
+      aliases.set(node.source,node.toJS(ast));
     }

Don't close this bug yet though, because this does need a better error.

@eemeli
Copy link
Owner

eemeli commented Apr 4, 2023

Closing with the fix committed in aa0a1d4.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants