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

[SharedUX] Replace antlr4ts dependency with official antlr package #172199

Closed
dej611 opened this issue Nov 29, 2023 · 6 comments · Fixed by #177211
Closed

[SharedUX] Replace antlr4ts dependency with official antlr package #172199

dej611 opened this issue Nov 29, 2023 · 6 comments · Fixed by #177211
Assignees
Labels
enhancement New value added to drive a business result Feature:ES|QL ES|QL related features in Kibana Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)

Comments

@dej611
Copy link
Contributor

dej611 commented Nov 29, 2023

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.

@dej611 dej611 added enhancement New value added to drive a business result Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) labels Nov 29, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@costin
Copy link
Member

costin commented Nov 29, 2023

👍 from my side. This helps reuse the ES|QL grammar across backed and UX.
Using the main library (ANTLR) makes it easy to pick security and performance improvements vs the fork which hasn't been updated in 3 years.

@eokoneyo
Copy link
Contributor

eokoneyo commented Dec 6, 2023

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.

@petrklapka
Copy link
Member

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.

@eokoneyo
Copy link
Contributor

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.

@eokoneyo
Copy link
Contributor

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

@eokoneyo eokoneyo linked a pull request Mar 4, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:ES|QL ES|QL related features in Kibana Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants