Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ryan-palmer-snc opened this issue Jul 5, 2024 · 1 comment

Comments

@ryan-palmer-snc
Copy link

ryan-palmer-snc commented Jul 5, 2024

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
Copy link
Author

ryan-palmer-snc commented Jul 5, 2024

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,
};

@ryan-palmer-snc ryan-palmer-snc changed the title Filesystem implementations must throw specific internal Error subclass in order to function properly Filesystem implementations must throw specific internal Error subclass in order to work, but that subclass is not exported properly Jul 5, 2024
@ryan-palmer-snc ryan-palmer-snc changed the title Filesystem implementations must throw specific internal Error subclass in order to work, but that subclass is not exported properly Filesystem implementations must throw specific Error subclass in order to work, but that subclass is not exported properly Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant