Change to return undefined, never test tree

Author: wooorm

lib/index.js

@@ -17,36 +17,30 @@ import {convert} from 'unist-util-is'
 /**
  * Change the given `tree` by removing all nodes that pass `test`.
  *
+ * `tree` itself is never tested.
  * The tree is walked in preorder (NLR), visiting the node itself, then its
  * head, etc.
  *
- * @template {Node} Tree
- *   Node kind.
- *
  * @overload
- * @param {Tree} node
+ * @param {Node} node
  * @param {Test} [test]
- * @returns {Tree | undefined}
+ * @returns {undefined}
  *
  * @overload
- * @param {Tree} node
+ * @param {Node} node
  * @param {Options | null | undefined} options
  * @param {Test} [test]
- * @returns {Tree | undefined}
+ * @returns {undefined}
  *
- * @param {Tree} tree
+ * @param {Node} tree
  *   Tree to change.
  * @param {Options | Test} options
  *   Configuration (optional).
  * @param {Test} [test]
  *   `unist-util-is` compatible test.
- * @returns {Tree | undefined}
- *   The given `tree` without nodes that pass `test`.
- *
- *   `undefined` is returned if `tree` itself didn’t pass the test or is
- *   cascaded away.
+ * @returns {undefined}
+ *   Nothing.
  */
-// To do: next major: don’t return `tree`.
 export function remove(tree, options, test) {
   const is = convert(test || options)
   let cascade = true
@@ -60,21 +54,20 @@ export function remove(tree, options, test) {
     cascade = options.cascade
   }
 
-  return preorder(tree)
+  preorder(tree)
 
   /**
    * Check and remove nodes recursively in preorder.
    * For each composite node, modify its children array in-place.
    *
-   * @template {Node} Kind
-   * @param {Kind} node
+   * @param {Node} node
    * @param {number | undefined} [index]
    * @param {Parent | undefined} [parent]
-   * @returns {Kind | undefined}
+   * @returns {boolean}
    */
   function preorder(node, index, parent) {
-    if (is(node, index, parent)) {
-      return undefined
+    if (node !== tree && is(node, index, parent)) {
+      return false
     }
 
     if ('children' in node && Array.isArray(node.children)) {
@@ -92,15 +85,15 @@ export function remove(tree, options, test) {
         }
 
         // Cascade delete.
-        if (cascade && !newChildIndex) {
-          return undefined
+        if (node !== tree && cascade && !newChildIndex) {
+          return false
         }
 
         // Drop other nodes.
         children.length = newChildIndex
       }
     }
 
-    return node
+    return true
   }
 }

readme.md

@@ -112,6 +112,7 @@ There is no default export.
 
 Change the given `tree` by removing all nodes that pass `test`.
 
+`tree` itself is never tested.
 The tree is walked in *[preorder][]* (NLR), visiting the node itself, then its
 head, etc.
 
@@ -126,10 +127,7 @@ head, etc.
 
 ###### Returns
 
-A changed given `tree`, without nodes that pass `test`.
-
-`undefined` is returned if `tree` itself didn’t pass the test or is cascaded
-away.
+Nothing (`undefined`).
 
 ### `Options`
 

test.js

@@ -24,12 +24,9 @@ test('remove', async function (t) {
     /** @type {Emphasis} */
     const tree = u('emphasis', children)
 
-    const next = remove(tree, {value: '2'})
+    remove(tree, {value: '2'})
 
     assert.deepEqual(tree, u('emphasis', [leaf1]))
-    assert.equal(next, tree)
-    assert.equal(next.children, children)
-    assert.equal(next.children[0], leaf1)
   })
 
   await t.test('should remove parent nodes', function () {
@@ -40,12 +37,9 @@ test('remove', async function (t) {
     /** @type {Root} */
     const tree = u('root', children)
 
-    const next = remove(tree, test)
+    remove(tree, test)
 
     assert.deepEqual(tree, u('root', [leaf2]))
-    assert.equal(next, tree)
-    assert.equal(next.children, children)
-    assert.equal(next.children[0], leaf2)
 
     /**
      * @param {Node} node
@@ -56,23 +50,25 @@ test('remove', async function (t) {
     }
   })
 
-  await t.test(
-    'should return `undefined` if root node is removed',
-    function () {
-      /** @type {Root} */
-      const tree = u('root', [u('emphasis', [u('text', '1')]), u('text', '2')])
-      const next = remove(tree, 'root')
+  await t.test('should not check root nodes', function () {
+    /** @type {Root} */
+    const tree = u('root', [u('emphasis', [u('text', '1')]), u('text', '2')])
 
-      assert.equal(next, undefined)
-    }
-  )
+    remove(tree, 'root')
+
+    assert.deepEqual(
+      tree,
+      u('root', [u('emphasis', [u('text', '1')]), u('text', '2')])
+    )
+  })
 
   await t.test('should cascade (remove) root nodes', function () {
     /** @type {Root} */
     const tree = u('root', [u('emphasis', [u('text', '1')]), u('text', '2')])
-    const next = remove(tree, 'text')
 
-    assert.equal(next, undefined)
+    remove(tree, 'text')
+
+    assert.deepEqual(tree, u('root', []))
   })
 
   await t.test(
@@ -122,9 +118,8 @@ test('remove', async function (t) {
   await t.test('should support `cascade = true`', function () {
     /** @type {Root} */
     const tree = u('root', [u('emphasis', [u('text', '1')]), u('text', '2')])
-    const next = remove(tree, {cascade: true}, 'text')
-
-    assert.equal(next, undefined)
+    remove(tree, {cascade: true}, 'text')
+    assert.deepEqual(tree, u('root', []))
   })
 
   await t.test('should support `cascade = false`', function () {
@@ -136,13 +131,9 @@ test('remove', async function (t) {
     /** @type {Root} */
     const tree = u('root', siblings)
 
-    const next = remove(tree, {cascade: false}, 'text')
+    remove(tree, {cascade: false}, 'text')
 
     assert.deepEqual(tree, u('root', [u('emphasis', [])]))
-    assert.equal(next, tree)
-    assert.equal(next.children, siblings)
-    assert.equal(next.children[0], node)
-    assert.equal(next.children[0].children, nodeChildren)
   })
 
   await t.test('should support the example from readme', function () {