Ts-morph: SourceFile.replaceWithText() would throw exception when previous code had issues, and had cached nodes

[Tundon]

Describe the bug

Version: 7.1.0 (tho it can be traced back 5.x or even earlier, I only tried until 5.x)

To Reproduce
here is a unit test case I created under ts-morph project, packages/ts-morph/src/tests/manipulation/manipulations/sourceFileReplaceTests.ts

import { Project } from "../../../Project";
import { SourceFile } from "../../../compiler";
import { expect } from "chai";

describe.only('SourceFile', () => {
  let project: Project, sourceFile: SourceFile;

  beforeEach(() => {
    project = new Project({ useInMemoryFileSystem: true })
  })

  describe('replaceWithText()', () => {
    describe('When source file had some issues', () => {
      beforeEach(() => {
        sourceFile = project.createSourceFile("test.ts", `
        interface Document {
          $save(): Promise<void>;
        }

        interface Product ex {
          name: string;
          description: string;
        }
        `);

      });

      describe('and had cached nodes', () => {
        beforeEach(() => {
          sourceFile.getInterfaceOrThrow("Document")
        });

        describe('invoke replaceWithText() with correct content', () => {
          it("should not throw", () => {
            expect(() => sourceFile.replaceWithText(`     
              interface Document {
                $save(): Promise<void>;
              }

              interface Product {
                name: string;
                description: string;
              }`)
            ).not.to.throw()
          });
        });
      });

    });
  });
});

the test case failed.

Expected behavior

the test case should pass.

[dsherret]

I think given that it's replacing the source file text, this could be fixed to work. It would be very hard to fix if only a single node was replaced within the source file text.

https://ts-ast-viewer.com/#code/JYOwLgpgTgZghgYwgAgCIHsEFcC2FzIDeAUMmeRQCQDOcAbhABQCUAXMgApTo7DUQAeOumAATAHwBuUhXIBfYjNnJQkWIhRd0orAjDIIADyJLlyEHDztqYKKADm0s+VERqCOwAcwwdCGu2Dk5mckA

[Tundon]

So a quick bug hunting led me to the code packages/ts-morph/src/manipulation/nodeHandlers/RangeParentHandler.ts around line 78, of function RangeParentHandler.handleNode(), it has

            // if the new node kinds and the old node kinds are the same, then don't forget the existing nodes
            if (oldNodes.length === newNodes.length && oldNodes.every((node, i) => node.kind === newNodes[i].kind)) {
                for (let i = 0; i < oldNodes.length; i++) {
                    const node = this.compilerFactory.getExistingNodeFromCompilerNode(oldNodes[i]);
                    if (node != null) {
                        node.forgetDescendants();
                        ^^^ this line would panic
                        this.compilerFactory.replaceCompilerNode(oldNodes[i], newNodes[i]);
                    }
                }
            }

and it panicked inside Node.forgetDescendants function.

    /**
     * Forgets the descendants of this node.
     */
    forgetDescendants() {
        for (const child of this._getChildrenInCacheIterator())
            child.forget();
            ^^^ here

        return this;
    }

A quick and dirty fix is to wrap the node.forgetDescendants() line with try ... catch, but I am not sure if that is the correct solution, as it may leak resources.

BTW, super quick response @dsherret !

[dsherret]

@Tundon fixed and published in 7.1.1 :)