feat!: Drop the map & seq .add() methods, override .push() instead

Author: eemeli

docs/04_documents.md

@@ -117,15 +117,14 @@ which is expected to always contain a `YAMLMap`, `YAMLSeq`, or `Scalar` value.
 ```js
 const doc = parseDocument('a: 1\nb: [2, 3]\n')
 doc.get('a') // 1
-doc.get() // YAMLMap { items: [Pair, Pair], ... }
 doc.get('b').has(0) // true
-doc.get('b').add(4) // -> doc.get('b').items.length === 3
+doc.get('b').push(4) // -> doc.get('b').items.length === 3
 doc.get('b').delete(1) // true
 doc.get('b').get(1) // 4
 ```
 
 In addition to the above, the document object also provides the same **accessor methods** as [collections](#collections), based on the top-level collection:
-`add`, `delete`, `get`, `has`, and `set`.
+`delete`, `get`, `has`, and `set`.
 
 #### `Document#toJS()`, `Document#toJSON()` and `Document#toString()`
 

docs/05_content_nodes.md

@@ -61,21 +61,19 @@ interface CollectionBase extends NodeBase {
   clone(schema?: Schema): this  // a deep copy of this collection
 }
 
-class YAMLMap<K = unknown, V = unknown> implements CollectionBase {
-  items: Pair<K, V>[]
-  add(pair: Pair<K, V> | { key: K; value: V }, overwrite?: boolean): void
+class YAMLMap<K = unknown, V = unknown> extends Array<Pair<K, V>> implements CollectionBase {
   delete(key: K): boolean
   get(key: K, keepScalar?: boolean): unknown
   has(key: K): boolean
+  push(...pairs: Pair<K, V>[]): number
   set(key: K, value: V): void
 }
 
-class YAMLSeq<T = unknown> implements CollectionBase {
-  items: T[]
-  add(value: T): void
+class YAMLSeq<T = unknown> extends Array<NodeOf<T>> implements CollectionBase {
   delete(key: number | Scalar<number>): boolean
   get(key: number | Scalar<number>, keepScalar?: boolean): unknown
   has(key: number | Scalar<number>): boolean
+  push(...items: Array<T | NodeOf<T>>): number
   set(key: number | Scalar<number>, value: T): void
 }
 ```
@@ -93,21 +91,21 @@ All of the collections provide the following accessor methods:
 
 | Method          | Returns   | Description                                                                                                                                                                                      |
 | --------------- | --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
-| add(value)      | `void`    | Adds a value to the collection. For `!!map` and `!!omap` the value must be a Pair instance, which must not have a key that already exists in the map.                                            |
 | delete(key)     | `boolean` | Removes a value from the collection. Returns `true` if the item was found and removed.                                                                                                           |
 | get(key)        | `Node`    | Returns value at `key`, or `undefined` if not found.                                                                                                                                             |
 | has(key)        | `boolean` | Checks if the collection includes a value with the key `key`.                                                                                                                                    |
+| push(...values) | `number`  | Adds values to the collection. For `!!map` and `!!omap` the value must be a Pair instance, which must not have a key that already exists in the map.                                             |
 | set(key, value) | `any`     | Sets a value in this collection. For `!!set`, `value` needs to be a boolean to add/remove the item from the set. When overwriting a `Scalar` value with a scalar, the original node is retained. |
 
 <!-- prettier-ignore -->
 ```js
 const doc = new YAML.Document({ a: 1, b: [2, 3] }) // { a: 1, b: [ 2, 3 ] }
-doc.add(doc.createPair('c', 4)) // { a: 1, b: [ 2, 3 ], c: 4 }
-doc.get('b').add(5)             // { a: 1, b: [ 2, 3, 5 ], c: 4 }
-doc.set('c', 42)                // { a: 1, b: [ 2, 3, 5 ], c: 42 }
-doc.get('c').set('x') // TypeError: doc.get(...).set is not a function
-doc.delete('c')                 // { a: 1, b: [ 2, 3, 5 ] }
-doc.get('b').delete(1)          // { a: 1, b: [ 2, 5 ] }
+doc.set('c', 4)         // { a: 1, b: [ 2, 3 ], c: 4 }
+doc.get('b').push(5)    // { a: 1, b: [ 2, 3, 5 ], c: 4 }
+doc.set('c', 42)        // { a: 1, b: [ 2, 3, 5 ], c: 42 }
+doc.get('c').set('x')   // TypeError: doc.get(...).set is not a function
+doc.delete('c')         // { a: 1, b: [ 2, 3, 5 ] }
+doc.get('b').delete(1)  // { a: 1, b: [ 2, 5 ] }
 
 doc.get('a') // Scalar { value: 1 }
 doc.get('b').get(1) // Scalar { value: 5 }
@@ -118,7 +116,7 @@ doc.get('b').has(0) // true
 
 For all of these methods, the keys may be nodes or their wrapped scalar values (i.e. `42` will match `Scalar { value: 42 }`).
 Keys for `!!seq` should be non-negative integers, or their string representations.
-`add()` and `set()` will internally call `doc.createNode()` to wrap the value.
+`set()` will internally call `doc.createNode()` to wrap the value.
 
 ## Alias Nodes
 
@@ -165,8 +163,8 @@ const map = doc.createNode({ balloons: 99 })
 //        key: Scalar { value: 'balloons' },
 //        value: Scalar { value: 99 } } ] }
 
-doc.add(map)
-doc.get(0, true).comment = ' A commented item'
+doc.value.push(map)
+doc.get(0).comment = ' A commented item'
 String(doc)
 // - some # A commented item
 // - values
@@ -197,8 +195,8 @@ To that end, you'll need to assign its return value to the `value` of a document
 <h4 style="clear:both"><code>doc.createAlias(node, name?): Alias</code></h4>
 
 ```js
-const alias = doc.createAlias(doc.get(1, true), 'foo')
-doc.add(alias)
+const alias = doc.createAlias(doc.get(1), 'foo')
+doc.value.push(alias)
 String(doc)
 // - some # A commented item
 // - &foo values
@@ -223,7 +221,7 @@ const doc = new Document([
   42,
   { including: 'objects', 3: 'a string' }
 ])
-doc.add(doc.createPair(1, 'a number'))
+doc.value.push(doc.createPair(1, 'a number'))
 
 doc.toString()
 // - some values

src/compose/resolve-block-map.ts

@@ -116,7 +116,7 @@ export function resolveBlockMap(
       offset = valueNode.range![2]
       const pair = new Pair(keyNode, valueNode)
       if (ctx.options.keepSourceTokens) pair.srcToken = collItem
-      map.push(pair)
+      map._push(pair)
     } else {
       // key with no value
       if (implicitKey)
@@ -131,7 +131,7 @@ export function resolveBlockMap(
       }
       const pair = new Pair(keyNode)
       if (ctx.options.keepSourceTokens) pair.srcToken = collItem
-      map.push(pair)
+      map._push(pair)
     }
   }
 

src/compose/resolve-block-seq.ts

@@ -50,7 +50,7 @@ export function resolveBlockSeq(
       : composeEmptyNode(ctx, props.end, start, null, props, onError)
     if (ctx.schema.compat) flowIndentCheck(bs.indent, value, onError)
     offset = node.range![2]
-    seq.push(node)
+    seq._push(node)
   }
   seq.range = [bs.offset, offset, commentEnd ?? offset]
   return seq

src/compose/resolve-flow-collection.ts

@@ -109,7 +109,7 @@ export function resolveFlowCollection(
       const valueNode = value
         ? composeNode(ctx, value, props, onError)
         : composeEmptyNode(ctx, props.end, sep, null, props, onError)
-      ;(coll as YAMLSeq).push(valueNode)
+      ;(coll as YAMLSeq)._push(valueNode)
       offset = valueNode.range![2]
       if (isBlock(value)) onError(valueNode.range!, 'BLOCK_IN_FLOW', blockMsg)
     } else {
@@ -193,14 +193,14 @@ export function resolveFlowCollection(
         const map = coll as YAMLMap
         if (mapIncludes(ctx, map, keyNode))
           onError(keyStart, 'DUPLICATE_KEY', 'Map keys must be unique')
-        map.push(pair)
+        map._push(pair)
       } else {
         const map = new YAMLMap(ctx.schema)
         map.flow = true
-        map.push(pair)
+        map._push(pair)
         const endRange = (valueNode ?? keyNode).range!
         map.range = [keyNode.range![0], endRange[1], endRange[2]]
-        ;(coll as YAMLSeq).push(map)
+        ;(coll as YAMLSeq)._push(map)
       }
       offset = valueNode ? valueNode.range![2] : valueProps.end
     }

src/doc/Document.ts

@@ -151,11 +151,6 @@ export class Document<
     return copy
   }
 
-  /** Adds a value to the document. */
-  add(value: any): void {
-    assertCollection(this.value).add(value)
-  }
-
   /**
    * Create a new `Alias` node, ensuring that the target `node` has the required anchor.
    *
@@ -238,14 +233,13 @@ export class Document<
    * @returns `true` if the item was found and removed.
    */
   delete(key: any): boolean {
-    return assertCollection(this.value).delete(key)
+    return isCollection(this.value) ? this.value.delete(key) : false
   }
 
   /**
    * Returns item at `key`, or `undefined` if not found.
    */
-  get(key?: any): Strict extends true ? Node | Pair | undefined : any {
-    if (key === undefined) return this.value
+  get(key: any): Strict extends true ? Node | Pair | undefined : any {
     return isCollection(this.value) ? this.value.get(key) : undefined
   }
 
@@ -261,7 +255,8 @@ export class Document<
    * boolean to add/remove the item from the set.
    */
   set(key: any, value: any): void {
-    assertCollection(this.value).set(key, value)
+    if (isCollection(this.value)) this.value.set(key, value)
+    else throw new Error('Expected a YAML collection as document value')
   }
 
   /**
@@ -342,8 +337,3 @@ export class Document<
     return stringifyDocument(this, options)
   }
 }
-
-function assertCollection(value: unknown) {
-  if (isCollection(value)) return value
-  throw new Error('Expected a YAML collection as document value')
-}

src/nodes/Collection.ts

@@ -30,9 +30,6 @@ export interface CollectionBase extends NodeBase {
    */
   clone(schema?: Schema): this
 
-  /** Adds a value to the collection. */
-  add(value: unknown): void
-
   /**
    * Removes a value from the collection.
    * @returns `true` if the item was found and removed.

src/nodes/YAMLMap.ts

@@ -122,42 +122,57 @@ export class YAMLMap<
     return copyCollection(this, schema)
   }
 
+  /** @private */
+  _push(pair: Pair<K, V>): void {
+    super.push(pair)
+  }
+
   /**
-   * Adds a key-value pair to the map.
+   * Adds new key-value pairs to the mapping, and returns its new length.
    *
-   * Using a key that is already in the collection overwrites the previous value.
+   * Added pairs must not have the same keys as ones previously set in the map.
    */
-  add(pair: Pair<K, V>): void {
-    if (!(pair instanceof Pair)) throw new TypeError('Expected a Pair')
+  push(...pairs: Pair<K, V>[]): number {
+    for (const pair of pairs) {
+      if (!(pair instanceof Pair)) {
+        const msg = `Expected a Pair, but found ${(pair as any).constructor?.name ?? pair}`
+        throw new TypeError(msg)
+      }
+      if (findPair(this, pair.key)) {
+        const msg = `Maps must not include duplicate keys: ${String(pair.key)}`
+        throw new Error(msg)
+      }
 
-    const prev = findPair(this, pair.key)
-    const sortEntries = this.schema?.sortMapEntries
-    if (prev) {
-      // For scalars, keep the old node & its comments and anchors
-      if (prev.value instanceof Scalar && pair.value instanceof Scalar)
-        prev.value.value = pair.value.value
-      else prev.value = pair.value
-    } else if (sortEntries) {
-      const i = this.findIndex(item => sortEntries(pair, item) < 0)
-      if (i === -1) this.push(pair)
-      else this.splice(i, 0, pair)
-    } else {
-      this.push(pair)
+      if (this.schema?.sortMapEntries) {
+        const sortEntries = this.schema.sortMapEntries
+        const i = this.findIndex(item => sortEntries(pair, item) < 0)
+        if (i === -1) super.push(pair)
+        else this.splice(i, 0, pair)
+      } else {
+        super.push(pair)
+      }
     }
+    return this.length
   }
 
+  /**
+   * Removes a value from the mapping.
+   * @returns `true` if the item was found and removed.
+   */
   delete(key: unknown): boolean {
     const it = findPair(this, key)
     if (!it) return false
     const del = this.splice(this.indexOf(it), 1)
     return del.length > 0
   }
 
+  /** Returns item at `key`, or `undefined` if not found.  */
   get(key: unknown): NodeOf<V> | undefined {
     const it = findPair(this, key)
     return it?.value ?? undefined
   }
 
+  /** Checks if the mapping includes a value with the key `key`.  */
   has(key: unknown): boolean {
     return !!findPair(this, key)
   }
@@ -170,17 +185,31 @@ export class YAMLMap<
     let pair: Pair
     if (isNode(key) && (value === null || isNode(value))) {
       pair = new Pair(key, value)
-    } else if (!this.schema) {
-      throw new Error('Schema is required')
     } else {
+      if (!this.schema) throw new Error('Schema is required')
       const nc = new NodeCreator(this.schema, {
         ...options,
         aliasDuplicateObjects: false
       })
       pair = nc.createPair(key, value)
       nc.setAnchors()
     }
-    this.add(pair as Pair<K, V>)
+
+    const prev = findPair(this, pair.key)
+    if (prev) {
+      const pv = pair.value as NodeOf<V>
+      // For scalars, keep the old node & its comments and anchors
+      if (prev.value instanceof Scalar && pv instanceof Scalar) {
+        Object.assign(prev.value, pv)
+      } else prev.value = pv
+    } else if (this.schema?.sortMapEntries) {
+      const sortEntries = this.schema.sortMapEntries
+      const i = this.findIndex(item => sortEntries(pair, item) < 0)
+      if (i === -1) super.push(pair as Pair<K, V>)
+      else this.splice(i, 0, pair as Pair<K, V>)
+    } else {
+      super.push(pair as Pair<K, V>)
+    }
   }
 
   /**