fix #320 #321
fix #320 #321
Conversation
jjkavalam
commented
Oct 17, 2021
jjkavalam
commented
- I have found this change to fix Add support for "value associated with the merge key is a sequence" #320
There was a problem hiding this comment.
Yeah, this seems valid. I got originally sidetracked by the examples here only including alias nodes or a sequence thereof as the value, but this should resolve the final issues with this.
One minor nitpick in the code that would be nice to resolve before merging.
src/nodes/addPairToJSMap.ts
Outdated
| @@ -16,6 +16,7 @@ export function addPairToJSMap( | |||
| { key, value }: Pair | |||
| ) { | |||
| if (ctx && ctx.doc.schema.merge && isMergeKey(key)) { | |||
| value = ctx && isAlias(value) ? value.resolve(ctx.doc) : value | |||
There was a problem hiding this comment.
| value = ctx && isAlias(value) ? value.resolve(ctx.doc) : value | |
| value = isAlias(value) ? value.resolve(ctx.doc) : value |
The previous line will check for ctx, so no need to do so here again. It looks like mergeToJSMap is also never called with an undefined ctx, so the checks there ought to be removed as well.
There was a problem hiding this comment.
@eemeli Thank you for taking the time. I have incorporated the suggestion as well as added a test. If all ok, will be happy to see this merged. Also when is this fix likely to be released ?
There was a problem hiding this comment.
I'll see about getting a new release out later this week. Got no strict schedule for that.
