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

Remove external dependency on Numeral.js #8583

Closed
Tracked by #163011
thomasneirynck opened this issue Oct 7, 2016 · 11 comments
Closed
Tracked by #163011

Remove external dependency on Numeral.js #8583

thomasneirynck opened this issue Oct 7, 2016 · 11 comments
Labels
Feature:FieldFormatters impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:enhancement Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) technical debt Improvement of the software architecture and operational architecture

Comments

@thomasneirynck
Copy link
Contributor

thomasneirynck commented Oct 7, 2016

We currently rely on numeral.js for number formatting (http://numeraljs.com/) and expose their pattern syntax to users in the field formatter settings

Remove this explicit external dependency. This would mean:

  • we explicitly document the pattern syntax API supported by Kibana
  • we maintain a test suite for it

This does not mean we need to remove numeral.js as an internal dependency, just that we take ownership of the pattern syntax.

See comments by @epixa here #8565 (comment).

@timroes timroes added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages and removed :Management DO NOT USE Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Nov 27, 2018
@nfarrell
Copy link

Just to add to this, the NumeralJS lib seems to be stale,

  • Last commit 18 months ago
  • 104 open pull requests

With how deeply NumeralJS is integrated into Kibana now, there's going to be a lot more effort than before in adding "en-ie" (Ireland, Euro) as an option and setting its value.

It's not a showstopper, just added effort for us.

@epixa epixa added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Dec 11, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@wylieconlon
Copy link
Contributor

I've done a lot of work to see if it's possible to replace numeral with Intl.NumberFormat, and the short answer is that it's blocked until 8.0 for various reasons. #53972

The short-term approach that I am taking instead is like you are suggesting here, to provide interfaces that express the same configuration as numeral, but in a way that doesn't depend on the library and can be changed later. So for example, the configuration { id: 'percent', minDecimals: 3 } might translate to the numeral pattern 0,0.000% but is also doable using Intl.NumberFormat({ style: 'percent', minimumFractionDigits: 3 }).

I have started implementing this in Lens in this future-facing way, but I think it's time to start discussing the deprecation plan for other apps. #56253

@wylieconlon wylieconlon added Team:AppArch and removed Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Feb 6, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@wylieconlon
Copy link
Contributor

wylieconlon commented Feb 6, 2020

Numeral.js patterns are a useful and concise abstraction, and the fact that we have been asking users to type them for years shows this. But Numeral has bugs and limitations, and it's time for us to go beyond. This comment will summarize the current state of things and talk about a plan.

Useful features of Numeral

  • Some users have become familiar with the syntax
  • Has the ability to choose decimal places, significant digits, scientific/accounting display
  • Bytes formatting scales the magnitude automatically

Limitations of Numeral

What needs to change?

a. @elastic/kibana-app-arch now owns field formatters and should take over maintenance.
b. The way Kibana uses numeral is not like a standalone library. We should move it into kibana/packages.
c. Because the way we use numeral is custom, we need to write Kibana documentation for formatting that we support. We should stop linking to 3rd-party docs. elastic/numeral-js#16
d. Backwards compatible pattern API, forward-facing API. This is how another Numeral fork did it, Numbro

Other related issue: #39211

@lukeelmers lukeelmers added this to To triage in kibana-app-arch via automation Mar 12, 2020
@lukeelmers lukeelmers added technical debt Improvement of the software architecture and operational architecture and removed Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages labels Jul 2, 2020
@wylieconlon
Copy link
Contributor

Just as an update, we've done 3/4 of the steps above. The remaining step is to define a future APi, and I've made a proposal in this comment

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Jun 16, 2021
@VijayDoshi
Copy link

@lukeelmers, is there work still happening on this?

@lukeelmers
Copy link
Member

@VijayDoshi Nothing happening on the core side, but @Dosant on app services was tracking this work in a meta issue here: #102239

He or @ppisljar @petrklapka may know more about how it is currently being prioritized

@ppisljar
Copy link
Member

we are not working on this at the moment due to other priorities

@petrklapka petrklapka added Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) and removed Team:AppServicesSv labels Dec 12, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@kertal
Copy link
Member

kertal commented Aug 9, 2023

We are closing this issue in order to provide better transparency of priorities. This issue won't be prioritized in the near future. We track it in #163011 for long term planning.

@kertal kertal closed this as completed Aug 9, 2023
kibana-app-arch automation moved this from To triage to Done in current release Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:FieldFormatters impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:enhancement Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) technical debt Improvement of the software architecture and operational architecture
Projects
kibana-app-arch
  
Done in current release
Development

No branches or pull requests