dsherret / ts-morph

TypeScript Compiler API wrapper for static analysis and programmatic code changes.
https://ts-morph.com
MIT License
4.87k stars 194 forks source link

Filesystem implementations must throw specific `Error` subclass in order to work, but that subclass is not exported properly #1552

Open ryan-palmer-snc opened 2 months ago

ryan-palmer-snc commented 2 months ago

Describe the bug

Version: 23.0.0

When implementing a FileSystemHost to pass to ts-morph, your readFileSync function must throw FileNotFoundError if the file does not exist. Otherwise project functions like addSourceFileAtPathIfExists will fail. This is because of this code from TransactionalFileSystem#readFileIfExistsSync in @ts-morph/common:

  readFileIfExistsSync(filePath: StandardizedFilePath, encoding: string | undefined) {
    if (this.#fileDeletedInMemory(filePath))
      return undefined;
    try {
      return this.readFileSync(filePath, encoding);
    } catch (err) {
      if (err instanceof errors.FileNotFoundError)
        return undefined;
      else
        throw err;
    }
  }

There is a note in the documentation that this function should throw FileNotFoundError which I noticed later, but from what I can tell, FileNotFoundError is not even exported from ts-morph so it's actually impossible to fix this without taking a dependency on @ts-morph/common.

Screenshot 2024-07-05 at 11 08 05 AM

To Reproduce

Implement a FileSystemHost which, when a file doesn't exist on readFileSync, throws any error other than FileNotFoundError. Inject this host into a new Project instance, and then try to add a new source file via project.addSourceFileAtPathIfExists(...).

Expected behavior

This is an implicit contract which FileSystemHost implementations must adhere to but (A) it is not obvious that this contract exists and (B) there seems to be no proper way to adhere to it.

I strongly feel that the runtime dependency on the specific subclass of error being thrown by a FileSystemHost implementation effectively sabotages the inversion of that dependency making it impractical to provide a custom implementation, and should be removed. Short of that, at the very least the issue where this error is apparently not exported correctly should be resolved.

ryan-palmer-snc commented 2 months ago

I am not sure why this line which attempts to export FileNotFoundError doesn't make its way to ts-morph.d.ts

const {
  InvalidOperationError,
  FileNotFoundError,
  ArgumentError,
  ArgumentNullOrWhitespaceError,
  ArgumentOutOfRangeError,
  ArgumentTypeError,
  BaseError,
  DirectoryNotFoundError,
  NotImplementedError,
  NotSupportedError,
  PathNotFoundError,
} = errors;
export {
  ArgumentError,
  ArgumentNullOrWhitespaceError,
  ArgumentOutOfRangeError,
  ArgumentTypeError,
  BaseError,
  DirectoryNotFoundError,
  FileNotFoundError,
  InvalidOperationError,
  NotImplementedError,
  NotSupportedError,
  PathNotFoundError,
};