elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.65k stars 8.23k forks source link

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

Closed dej611 closed 8 months ago

dej611 commented 11 months ago

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.

elasticmachine commented 11 months ago

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

costin commented 11 months ago

👍 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 commented 11 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

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