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

Export errors from main #402

Merged
merged 4 commits into from
Aug 26, 2018
Merged

Export errors from main #402

merged 4 commits into from
Aug 26, 2018

Conversation

rhys-vdw
Copy link
Contributor

Export errors from module.

It's not possible to access code without using any. Export error classes for use in type guards.

import { FileNotFoundError } from "ts-simple-ast"

generate(paths)
  .then(() => {
    console.log('Done!')
  })
  .catch((error: Error) => {
    if (error.code === 'ENOENT') // [ts] Property 'code' does not exist on type 'Error'.
      console.error(error.message)

    if (error instanceof FileNotFoundError) // Good!
      console.error(error.message)
  })

@coveralls
Copy link

coveralls commented Aug 22, 2018

Coverage Status

Coverage decreased (-0.01%) to 94.778% when pulling 35d3d65 on rhys-vdw:export-error into e82734b on dsherret:dev.

@dsherret
Copy link
Owner

I'm currently using these errors as internal helpers in a way and don't want to export all of them (exporting the folder also exports the actual errors/helpers.ts file, which should remain internal).

I'm thinking only the FileNotFoundError, DirectoryNotFoundError, and PathNotFoundError classes should be exported, but then I'm not sure about that at the moment... need to think it over more (leaning towards making FileNotFoundError and DirectoryNotFoundError inherit from PathNotFoundError and then only expose PathNotFoundError). What do you think?

@rhys-vdw
Copy link
Contributor Author

Sounds fair, I'm actually not that familiar with all the other exceptions. The rule I would use is that any that can be thrown by the public API are part of the public API and should be exported. Any that are thrown and caught internally needn't be.

WRT inheritance hierarchies, I don't see much value in it. It seems like indirection. For example I got a FileNotFound error that I wanted to handle, so looked for the corresponding export to match it. It would not be obvious that I should have imported an error with a different name.

What's the point of subclassing if you do not export the errors?

- FileNotFoundError and DirectoryNotFoundError inherit from PathNotFoundError in order to allow just checking for PathNotFoundError.
@dsherret dsherret changed the base branch from master to dev August 26, 2018 03:23
@dsherret dsherret merged commit b719ed8 into dsherret:dev Aug 26, 2018
@dsherret
Copy link
Owner

dsherret commented Aug 26, 2018

Good points! I exported all of them and will make a release soon (probably tomorrow).

@rhys-vdw rhys-vdw deleted the export-error branch August 26, 2018 04:01
dsherret pushed a commit that referenced this pull request May 14, 2019
* Export errors from main

* Move classes to separate folder so they can be exported on their own.

* Constructors internal.

* FileNotFoundError and DirectoryNotFoundError inherit from PathNotFoundError in order to allow just checking for PathNotFoundError.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants