-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[SharedUX] Replace antlr4ts dependency with official antlr package #172199
Comments
Pinging @elastic/appex-sharedux (Team:SharedUX) |
👍 from my side. This helps reuse the ES|QL grammar across backed and UX. |
Hey @dej611 I suppose given that antlr now offers a typescript target, it'd be ideal to use the library directly so we can benefit from updates and definitely have ES|QL leverage the benefits that the upgrade would yield, that been said we'd have to evaluate how this fits into work shared UX already has planned and circle back to you. |
I've added this as an "epic" to our quarterly planning board for visibility. The engineers want to do a time boxed dive into this before estimating a total replacement, so we will begin with a 1 week timebox to learn more and estimate LOE. |
Update This task has been picked up and the shared UX team will be able to provide an estimate for the LOE required to switch to the official package within a week. |
Update A PoC was created to explore the effort required to migrate away from antlr4ts to the official antlr package here, the migration is under way with couple of validations left to be done to ascertain parity in functionality and that the switch does not result in regressions |
Describe the feature:
The offiicial ANTLR package supports Typescript and it is more mantained than the fork we're using for Kibana: https://github.com/antlr/antlr4/blob/dev/doc/typescript-target.md
Is there any particular reason to use that fork rather than the official package?
One strong case we have to switch is the "case insensitivity" support in 4.10 which is lacking in ANTLR4ts (due to this issue), that would make us adopt exactly the same grammar as in ES for ES|QL.
The text was updated successfully, but these errors were encountered: