Merge pull request #250 from eemeli/solo-pair

Make Pair not extend NodeBase; drop its prop forwarding

docs/05_content_nodes.md

@@ -46,7 +46,7 @@ On the other hand, `!!int` and `!!float` stringifiers will take `format` into ac
 ## Collections
 
 ```js
-class Pair<K = unknown, V = unknown> extends NodeBase {
+class Pair<K = unknown, V = unknown> {
   key: K    // When parsed, key and value are always
   value: V  // Node or null, but can be set to anything
 }

src/compose/composer.ts

@@ -1,7 +1,7 @@
 import { Directives } from '../doc/directives.js'
 import { Document } from '../doc/Document.js'
 import { ErrorCode, YAMLParseError, YAMLWarning } from '../errors.js'
-import { isCollection } from '../nodes/Node.js'
+import { isCollection, isPair } from '../nodes/Node.js'
 import {
   defaultOptions,
   DocumentOptions,
@@ -99,7 +99,8 @@ export class Composer {
       } else if (afterEmptyLine || doc.directives.marker || !dc) {
         doc.commentBefore = comment
       } else if (isCollection(dc) && !dc.flow && dc.items.length > 0) {
-        const it = dc.items[0]
+        let it = dc.items[0]
+        if (isPair(it)) it = it.key
         const cb = it.commentBefore
         it.commentBefore = cb ? `${comment}\n${cb}` : comment
       } else {

src/nodes/Collection.ts

@@ -1,7 +1,6 @@
 import { createNode } from '../doc/createNode.js'
 import type { Schema } from '../schema/Schema.js'
-import { isCollection, isNode, isScalar, NodeBase, NODE_TYPE } from './Node.js'
-import type { Pair } from './Pair.js'
+import { isCollection, isPair, isScalar, NodeBase, NODE_TYPE } from './Node.js'
 
 export function collectionFromPath(
   schema: Schema,
@@ -143,8 +142,8 @@ export abstract class Collection extends NodeBase {
 
   hasAllNullValues(allowScalar?: boolean) {
     return this.items.every(node => {
-      if (!node || isNode(node)) return false
-      const n = (node as Pair).value
+      if (!isPair(node)) return false
+      const n = node.value
       return (
         n == null ||
         (allowScalar &&

src/nodes/Pair.ts

@@ -2,8 +2,7 @@ import { createNode, CreateNodeContext } from '../doc/createNode.js'
 import { StringifyContext } from '../stringify/stringify.js'
 import { stringifyPair } from '../stringify/stringifyPair.js'
 import { addPairToJSMap } from './addPairToJSMap.js'
-import { isNode, NodeBase, PAIR } from './Node.js'
-import { Scalar } from './Scalar.js'
+import { NODE_TYPE, PAIR } from './Node.js'
 import { ToJSContext } from './toJS.js'
 
 export function createPair(
@@ -16,46 +15,19 @@ export function createPair(
   return new Pair(k, v)
 }
 
-export class Pair<K = unknown, V = unknown> extends NodeBase {
+export class Pair<K = unknown, V = unknown> {
+  readonly [NODE_TYPE]: symbol
+
   /** Always Node or null when parsed, but can be set to anything. */
   key: K
 
   /** Always Node or null when parsed, but can be set to anything. */
   value: V | null
 
   constructor(key: K, value: V | null = null) {
-    super(PAIR)
+    Object.defineProperty(this, NODE_TYPE, { value: PAIR })
     this.key = key
     this.value = value
-
-    // TS doesn't allow for accessors to override properties
-    // https://github.com/microsoft/TypeScript/pull/33509
-    Object.defineProperties(this, {
-      commentBefore: {
-        get: () => (isNode(this.key) ? this.key.commentBefore : undefined),
-        set: (cb: string | null) => {
-          if (this.key == null) this.key = (new Scalar(null) as unknown) as K
-          if (isNode(this.key)) this.key.commentBefore = cb
-          else {
-            const msg =
-              'Pair.commentBefore is an alias for Pair.key.commentBefore. To set it, the key must be a Node.'
-            throw new Error(msg)
-          }
-        }
-      },
-      spaceBefore: {
-        get: () => (isNode(this.key) ? this.key.spaceBefore : undefined),
-        set: (sb: boolean) => {
-          if (this.key == null) this.key = (new Scalar(null) as unknown) as K
-          if (isNode(this.key)) this.key.spaceBefore = sb
-          else {
-            const msg =
-              'Pair.spaceBefore is an alias for Pair.key.spaceBefore. To set it, the key must be a Node.'
-            throw new Error(msg)
-          }
-        }
-      }
-    })
   }
 
   toJSON(_?: unknown, ctx?: ToJSContext): ReturnType<typeof addPairToJSMap> {

src/schema/types.ts

@@ -100,7 +100,7 @@ export interface CollectionTag extends TagBase {
    * If returning a non-`Node` value, the output will be wrapped as a `Scalar`.
    */
   resolve(
-    value: YAMLMap | YAMLSeq,
+    value: YAMLMap.Parsed | YAMLSeq.Parsed,
     onError: (message: string) => void,
     options: ParseOptions
   ): unknown

src/schema/yaml-1.1/pairs.ts

@@ -1,13 +1,16 @@
 import type { CreateNodeContext } from '../../doc/createNode.js'
-import type { Schema } from '../../schema/Schema.js'
-import { isMap, isPair, isSeq } from '../../nodes/Node.js'
+import { isMap, isPair, isSeq, ParsedNode } from '../../nodes/Node.js'
 import { createPair, Pair } from '../../nodes/Pair.js'
+import { Scalar } from '../../nodes/Scalar.js'
 import { YAMLMap } from '../../nodes/YAMLMap.js'
 import { YAMLSeq } from '../../nodes/YAMLSeq.js'
+import type { Schema } from '../../schema/Schema.js'
 import type { CollectionTag } from '../types.js'
 
 export function resolvePairs(
-  seq: YAMLSeq | YAMLMap,
+  seq:
+    | YAMLSeq.Parsed<ParsedNode | Pair<ParsedNode, ParsedNode | null>>
+    | YAMLMap.Parsed,
   onError: (message: string) => void
 ) {
   if (isSeq(seq)) {
@@ -17,21 +20,24 @@ export function resolvePairs(
       else if (isMap(item)) {
         if (item.items.length > 1)
           onError('Each pair must have its own sequence indicator')
-        const pair = item.items[0] || new Pair(null)
+        const pair =
+          item.items[0] || new Pair(new Scalar(null) as Scalar.Parsed)
         if (item.commentBefore)
-          pair.commentBefore = pair.commentBefore
-            ? `${item.commentBefore}\n${pair.commentBefore}`
+          pair.key.commentBefore = pair.key.commentBefore
+            ? `${item.commentBefore}\n${pair.key.commentBefore}`
             : item.commentBefore
-        if (item.comment)
-          pair.comment = pair.comment
-            ? `${item.comment}\n${pair.comment}`
+        if (item.comment) {
+          const cn = pair.value || pair.key
+          cn.comment = cn.comment
+            ? `${item.comment}\n${cn.comment}`
             : item.comment
+        }
         item = pair
       }
       seq.items[i] = isPair(item) ? item : new Pair(item)
     }
   } else onError('Expected a sequence for this tag')
-  return seq as YAMLSeq<Pair>
+  return seq as YAMLSeq.Parsed<Pair<ParsedNode, ParsedNode | null>>
 }
 
 export function createPairs(

src/stringify/stringifyCollection.ts

@@ -28,57 +28,76 @@ export function stringifyCollection(
   const inFlow = flow || ctx.inFlow
   if (inFlow) itemIndent += indentStep
   ctx = Object.assign({}, ctx, { indent: itemIndent, inFlow, type: null })
-  let chompKeep = false
-  let hasItemWithNewLine = false
+
+  let singleLineOutput = true
+  let chompKeep = false // flag for the preceding node's status
   const nodes = items.reduce((nodes: StringifyNode[], item, i) => {
     let comment: string | null = null
-    if (isNode(item) || isPair(item)) {
+    if (isNode(item)) {
       if (!chompKeep && item.spaceBefore) nodes.push({ comment: true, str: '' })
-
       if (item.commentBefore) {
         // This match will always succeed on a non-empty string
         for (const line of item.commentBefore.match(/^.*$/gm) as string[])
           nodes.push({ comment: true, str: `#${line}` })
       }
+      if (item.comment) {
+        comment = item.comment
+        singleLineOutput = false
+      }
+    } else if (isPair(item)) {
+      const ik = isNode(item.key) ? item.key : null
+      if (ik) {
+        if (!chompKeep && ik.spaceBefore) nodes.push({ comment: true, str: '' })
+        if (ik.commentBefore) {
+          // This match will always succeed on a non-empty string
+          for (const line of ik.commentBefore.match(/^.*$/gm) as string[])
+            nodes.push({ comment: true, str: `#${line}` })
+        }
+        if (ik.comment) singleLineOutput = false
+      }
 
-      if (item.comment) comment = item.comment
-
-      const pair = item as any // Apply guards manually in the following
-      if (
-        inFlow &&
-        ((!chompKeep && item.spaceBefore) ||
-          item.commentBefore ||
-          item.comment ||
-          (pair.key && (pair.key.commentBefore || pair.key.comment)) ||
-          (pair.value && (pair.value.commentBefore || pair.value.comment)))
-      )
-        hasItemWithNewLine = true
+      if (inFlow) {
+        const iv = isNode(item.value) ? item.value : null
+        if (iv) {
+          if (iv.comment) comment = iv.comment
+          if (iv.comment || iv.commentBefore) singleLineOutput = false
+        } else if (item.value == null && ik && ik.comment) {
+          comment = ik.comment
+        }
+      }
     }
+
     chompKeep = false
     let str = stringify(
       item,
       ctx,
       () => (comment = null),
       () => (chompKeep = true)
     )
-    if (inFlow && !hasItemWithNewLine && str.includes('\n'))
-      hasItemWithNewLine = true
     if (inFlow && i < items.length - 1) str += ','
     str = addComment(str, itemIndent, comment)
     if (chompKeep && (comment || inFlow)) chompKeep = false
     nodes.push({ comment: false, str })
     return nodes
   }, [])
+
   let str: string
   if (nodes.length === 0) {
     str = flowChars.start + flowChars.end
   } else if (inFlow) {
     const { start, end } = flowChars
     const strings = nodes.map(n => n.str)
+    let singleLineLength = 2
+    for (const node of nodes) {
+      if (node.comment || node.str.includes('\n')) {
+        singleLineOutput = false
+        break
+      }
+      singleLineLength += node.str.length + 2
+    }
     if (
-      hasItemWithNewLine ||
-      strings.reduce((sum, str) => sum + str.length + 2, 2) >
-        Collection.maxFlowStringSingleLineLength
+      !singleLineOutput ||
+      singleLineLength > Collection.maxFlowStringSingleLineLength
     ) {
       str = start
       for (const s of strings) {

src/stringify/stringifyPair.ts

@@ -5,7 +5,7 @@ import { addComment } from './addComment.js'
 import { stringify, StringifyContext } from './stringify.js'
 
 export function stringifyPair(
-  { comment, key, value }: Readonly<Pair>,
+  { key, value }: Readonly<Pair>,
   ctx: StringifyContext,
   onComment?: () => void,
   onChompKeep?: () => void
@@ -30,7 +30,7 @@ export function stringifyPair(
   let explicitKey =
     !simpleKeys &&
     (!key ||
-      (keyComment && value == null) ||
+      (keyComment && value == null && !ctx.inFlow) ||
       isCollection(key) ||
       (isScalar(key)
         ? key.type === Scalar.BLOCK_FOLDED || key.type === Scalar.BLOCK_LITERAL
@@ -41,11 +41,12 @@ export function stringifyPair(
     implicitKey: !explicitKey && (simpleKeys || !allNullValues),
     indent: indent + indentStep
   })
+  let keyCommentDone = false
   let chompKeep = false
   let str = stringify(
     key,
     ctx,
-    () => (keyComment = null),
+    () => (keyCommentDone = true),
     () => (chompKeep = true)
   )
 
@@ -57,29 +58,21 @@ export function stringifyPair(
     explicitKey = true
   }
 
-  if (
-    (allNullValues && (!simpleKeys || ctx.inFlow)) ||
-    (value == null && (explicitKey || ctx.inFlow))
-  ) {
-    str = addComment(str, ctx.indent, keyComment)
-    if (comment) {
-      if (keyComment && !comment.includes('\n'))
-        str += `\n${ctx.indent || ''}#${comment}`
-      else str = addComment(str, ctx.indent, comment)
-      if (onComment) onComment()
-    } else if (chompKeep && !keyComment && onChompKeep) onChompKeep()
-    return ctx.inFlow && !explicitKey ? str : `? ${str}`
+  if (ctx.inFlow) {
+    if (allNullValues || value == null) {
+      if (keyCommentDone && onComment) onComment()
+      return explicitKey ? `? ${str}` : str
+    }
+  } else if ((allNullValues && !simpleKeys) || (value == null && explicitKey)) {
+    if (keyCommentDone) keyComment = null
+    if (chompKeep && !keyComment && onChompKeep) onChompKeep()
+    return addComment(`? ${str}`, ctx.indent, keyComment)
   }
 
+  if (keyCommentDone) keyComment = null
   str = explicitKey
     ? `? ${addComment(str, ctx.indent, keyComment)}\n${indent}:`
     : addComment(`${str}:`, ctx.indent, keyComment)
-  if (comment) {
-    if (keyComment && !explicitKey && !comment.includes('\n'))
-      str += `\n${ctx.indent || ''}#${comment}`
-    else str = addComment(str, ctx.indent, comment)
-    if (onComment) onComment()
-  }
 
   let vcb = ''
   let valueComment = null
@@ -94,7 +87,7 @@ export function stringifyPair(
     value = doc.createNode(value)
   }
   ctx.implicitKey = false
-  if (!explicitKey && !keyComment && !comment && isScalar(value))
+  if (!explicitKey && !keyComment && isScalar(value))
     ctx.indentAtStart = str.length + 1
   chompKeep = false
   if (
@@ -110,19 +103,28 @@ export function stringifyPair(
     // If indentSeq === false, consider '- ' as part of indentation where possible
     ctx.indent = ctx.indent.substr(2)
   }
+
+  let valueCommentDone = false
   const valueStr = stringify(
     value,
     ctx,
-    () => (valueComment = null),
+    () => (valueCommentDone = true),
     () => (chompKeep = true)
   )
   let ws = ' '
-  if (vcb || keyComment || comment) {
+  if (vcb || keyComment) {
     ws = `${vcb}\n${ctx.indent}`
   } else if (!explicitKey && isCollection(value)) {
     const flow = valueStr[0] === '[' || valueStr[0] === '{'
     if (!flow || valueStr.includes('\n')) ws = `\n${ctx.indent}`
   } else if (valueStr[0] === '\n') ws = ''
-  if (chompKeep && !valueComment && onChompKeep) onChompKeep()
-  return addComment(str + ws + valueStr, ctx.indent, valueComment)
+
+  if (ctx.inFlow) {
+    if (valueCommentDone && onComment) onComment()
+    return str + ws + valueStr
+  } else {
+    if (valueCommentDone) valueComment = null
+    if (chompKeep && !valueComment && onChompKeep) onChompKeep()
+    return addComment(str + ws + valueStr, ctx.indent, valueComment)
+  }
 }