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

Antlr4 JS/TS runtime DefaultErrorStrategy is undefined #4287

Closed
matthew-dean opened this issue May 27, 2023 · 6 comments
Closed

Antlr4 JS/TS runtime DefaultErrorStrategy is undefined #4287

matthew-dean opened this issue May 27, 2023 · 6 comments

Comments

@matthew-dean
Copy link

matthew-dean commented May 27, 2023

DefaultErrorStrategy was exported in the TS runtime as of the latest release (4.13.0). However, doing the following:

import {
  DefaultErrorStrategy
} from 'antlr4'

class CssParserErrorStrategy extends DefaultErrorStrategy {

}

...produces this error:

TypeError: Class extends value undefined is not a constructor or null

Executing console.log(DefaultErrorStrategy) before this line returns a value of undefined.

This probably wasn't caught due to the following which I've mentioned:

  1. The TS runtime is not properly type-checked.
  2. Even if it were, (nearly) all the .d.ts files are defined incorrectly, meaning it might pass types that don't represent actual runtime values

There's no workaround because of #2 above, meaning the source file cannot be easily referenced directly, because it has a mis-matched .d.ts file.

Tracking down the issue

I traced the files in terms of the actual .js runtime.

  1. DefaultErrorStrategy has a default export
  2. In error/index.js, the default export is imported, then referenced as a property on the default export of error/index.js. This is weird, but should work.
  3. The parent index.js file imports the default error export as error, and exports the default as error.
  4. Ah, okay. There is no named value DefaultErrorStrategy in the index.node.js or index.web.js exports, but all the named exports are exported in index.d.ts, leading TypeScript to believe there is an export named DefaultErrorStrategy, but it doesn't exist. This is, again, because nearly none of the TypeScript .d.ts files actually represent the files they're supposed to represent, and are manually managed. A better, more maintainable approach would probably be to eliminate all hand-coded .d.ts files and instead represent types using JSDoc comments, which could automatically export .d.ts files in a build step.

The interesting thing is that, technically, DefaultErrorStrategy is exported on an error object; but the problem is that TypeScript has no knowledge of it.

@mike-lischke
Copy link
Member

@matthew-dean It's actually the other way around: TS knows the type and imports it successfully, but running that code leads to an error that DefaultErrorStrategy is not defined.

However, I cannot get it to work by adding an export of DefaultErrorStrategy to either file (index.node.js and index.web.js). Have you solved this issue somehow?

It's a PITA that this (and other issues) are being ignored. I'm currently massaging the JS target to be able to collect some performance stats. This is taking hours ...

@matthew-dean
Copy link
Author

matthew-dean commented Aug 25, 2023

@mike-lischke I struggled and struggled with the JavaScript / TypeScript implementation of Antlr because I wanted a more abstracted grammar, but ultimately I just found so many issues like this that I went back to working with Chevrotain. 🙁 (Chevrotain is actually really easy to use, but any grammar you write is JS/TS-specific.) At this point, I really don't think it's truthful to say Antlr has JS / TS support. After getting the book (because that's the only support I could get from anyone regarding how to use Antlr -> buy this specific book), I really feel that Antlr is a parsing library for Java and Java alone. If you're not using Java, it seems to either have a sub-set of functionality, or in the case of JS/TS, broken functionality. I hope that doesn't offend anyone - that's just my experience!

@mike-lischke
Copy link
Member

I understand your disappointment, but it's really only the current JS target which fails so badly.

FYI I have a running wasm TS runtime already and do performance measuring right now. However, that's probably no longer interesting for you given you left the ANTLR world.

I guess you can close this ticket then.

@ericvergnaud
Copy link
Contributor

ericvergnaud commented Aug 25, 2023

Looks like we're simply missing export * from './DefaultErrorStrategy'; in error.index.t.ts ?
Anyone in need of this could try that out and provide a PR if it fixes the issue ?

@mike-lischke
Copy link
Member

Thanks @ericvergnaud for the hint, but for my benchmarks this is not needed. I have no intention to use the current JS target further than for that.

@ericvergnaud
Copy link
Contributor

Fixed by #4278

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

3 participants