Why alias resolution context is not propagated to merge key resolution?

[devingfx]

Hi!

( Your YAML lib is awesome and the only one to provide AST out there! So first: Thank you ! )

I am cooking a plugin to the library to be able to add a custom alias resolution function in case the alias is missing its corresponding anchored node.
It work well in general, but it relies on puting the missing nodes (usually generated) in ctx.anchors for later resolution to use cached values...

I wonder why the merge key part do not use/propagate the ctx while resolving alias for merge keys?

Here:

yaml/src/schema/yaml-1.1/merge.ts

Line 46 in 6a6f8fb

value = ctx && isAlias(value) ? value.resolve(ctx.doc) : value

and here:

yaml/src/schema/yaml-1.1/merge.ts

Line 58 in 6a6f8fb

const source = ctx && isAlias(value) ? value.resolve(ctx.doc) : value

Those should be

value.resolve(ctx.doc, ctx)

, doesn't they?

Is there a particular reason (I'm missing) why those resolution does not use the context ?

Thx

PS: My plugin would mainly be used to parse file path from missing alias and load the file for it to be included inplace (maybe targeting data with JSONPath). While I could not use ctx.anchor, the cached strategy would be more efficient in case big files are loaded and used several times...

Example: this loads & parse 2 times the same common.yaml file

config:
  <<: *./configs/common.yaml
  foo: bar override
database:
  <<: *./configs/common.yaml

While this example does not (because of ctx)

config: *./configs/common.yaml
database: *./configs/common.yaml

but lacks of a way to override/add pairs to the map...

[devingfx]

Here is the code...
Some explainations:

• The custom resolveMissingAlias function is given in parsing options that is dropped as is on Document.options
• It have to return a CST node directly, to let the toJS do its job resolving stuff and potentially nested missing alias the same way recursively.
• toJS.toJS is not easily accessible so I used the onCreate pattern to update the anchor's value when it would be resolved.
• The potential file loading is hidden here, it is done in resolveMissingAlias, it loads the file, uses parseDocument to get a CST node from it and return the result.

import * as YAML from 'npm:yaml@2.8.1'
const { Alias } = YAML
// import toJS from '/home/dig/.cache/deno/npm/registry.npmjs.org/yaml/2.8.1/dist/nodes/toJS.js'

const _resolve = Alias.prototype.resolve
Alias.prototype.resolve = function( doc, ctx )
{
	const _found = _resolve.call( this, doc, ctx )
	if( !_found && typeof doc.options.resolveMissingAlias == 'function' && ctx )	// no ctx when simply counting aliases (so do not resolve here)
	{
		console.log('alias trap', this, doc.options.resolveMissingAlias )
		// must return a CST node with an anchor
		const found = doc.options.resolveMissingAlias( this, doc, ctx )
		found.anchor = this.source
		
		// ctx.anchors.set( found, {aliasCount: 0 , count: 1, res: toJS.toJS(found, '', ctx)} )
		// @TODO x try avoid toJS.toJS with onCreate pattern @see toJS.js
		
		// @see toJS.js
		const data = { aliasCount: 0, count: 1, res: undefined }
		ctx.anchors.set(found, data)
		ctx.onCreate = res => {
			data.res = res
			delete ctx.onCreate
		}
		const res = found.toJSON('', ctx)
		if (ctx.onCreate)
			ctx.onCreate(res)
		// end toJS.js
		
		// @TODO must sort by dependencies
		ctx.aliasResolveCache.unshift( found )
		return found
	}
	return _found
}

So as you may understand that the CST node returned from resolveMissingAlias is not part of the document, it is only added in ctx.anchors for alias resolution. This way the original document is not poluted with some additional included nodes like it would be if added like so:

CUSTOM_SPECIAL_KEY_TO_BE_REMOVED: &./configs/common.yaml
  <the content of the file>
config: *./configs/common.yaml
database: *./configs/common.yaml

[eemeli]

Sorry, had managed to miss the notification for this issue somehow.

Yeah, that looks like a bug with a relatively straightforward fix. It would be really nice to include a test case for this, which probably needs a custom Alias resolver as you're doing. Would you be interested in submitting a PR?

[devingfx]

Yep!
Sorry for late reply, I was planning to have a look to this next week (because my agenda is quite full this week), but you did the job already! 🤷‍♂️
BTW I would not have pointed out those performance improvment you did...

Thx for this !