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

docs: exception handler #102

Closed
hildjj opened this issue Apr 13, 2023 · 4 comments · Fixed by #103
Closed

docs: exception handler #102

hildjj opened this issue Apr 13, 2023 · 4 comments · Fixed by #103

Comments

@hildjj
Copy link
Contributor

hildjj commented Apr 13, 2023

I'm not sure this works as intended. I get a TypeScript error from the exception handler. The goal in that handler is being able to call format() with type safety.

import { PeggySyntaxError, parse } from './arithmetics';

try {
    const sampleOutput = parse('my sample...');
} catch (ex: PeggySyntaxError) {
    // Handle parsing error
    // [...]
}
@pjmolina
Copy link
Contributor

pjmolina commented Apr 14, 2023

Hi. It looks like the exported of the type in arithmetics.ts is not property exported:

export const PeggySyntaxError = peggyParser.SyntaxError as typeof _PeggySyntaxError;

I this this fix will work:

export type PeggySyntaxError = _PeggySyntaxError;

Can you confirm if this @hildjj ? cc @siefkenj

pjmolina added a commit that referenced this issue Apr 14, 2023
@pjmolina pjmolina mentioned this issue Apr 14, 2023
@siefkenj
Copy link
Contributor

Ah. This is very complicated, it seems, but I think I found a solution.

The issue is PeggySyntaxError is not just a type, it's also a class that can be constructed.

@pjmolina Your fix is correct, but it needs to be exported both ways. That is, the generated code should be

export const PeggySyntaxError = peggyParser.SyntaxError as typeof _PeggySyntaxError;
export type PeggySyntaxError = _PeggySyntaxError;

so that the constructor and the type can both be exported.

I think DefaultTracer will also have the same issue since it is exported the same way as PeggySyntaxError.

@pjmolina
Copy link
Contributor

Indeed, good catch @siefkenj
Let me fix the PR.

@pjmolina
Copy link
Contributor

I think PR #103 is ready to fix this.

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

Successfully merging a pull request may close this issue.

3 participants