[GitHub] kristw commented on issue #6470: [WIP] Use @superset-ui/number-format and @superset-ui/time-format for formatting.

[GitBox <gi...@apache.org>]

kristw commented on issue #6470: [WIP] Use @superset-ui/number-format and @superset-ui/time-format for formatting. 
URL: https://github.com/apache/incubator-superset/pull/6470#issuecomment-443367285
 
 
   @michellethomas This was because there are a few ways to use `d3` formatting in superset master.
   
   - Native `d3.format(format)`. 
   - Superset's `d3format(format, value)` in `utils.js`
   - Superset's `d3FormatPreset(format)` in `utils.js`
   
   Prior to `0.29`, all of these functions throw error immediately for invalid `d3` format string. PR #6386 fixes the latter two to handle invalid format strings and show `ERROR` instead (with unit tests). 
   
   However
   - The direct calls to `d3.format` are still vulnerable.
   - There was a case at Airbnb with `Infinity` value that crash Table vis, which was not handled by #6386 because the issue is with value, not the format string.
   
   To resolve these, in this PR, all the three calls above are replaced with `getNumberFormatter(format)` or `formatNumber(format, value)` from `@superset-ui/number-format`. These two functions can handle both invalid **formats** and **values** and has been written with unit tests. 
   - `formatNumber('.abc', 200)` => `200 (Invalid format: .abc)`
   - `formatNumber('.2f', Infinity)` => `∞`
   
   See unit tests
   - [NumberFormatter tests](https://github.com/apache-superset/superset-ui/blob/master/packages/superset-ui-number-format/test/NumberFormatter.test.js)
   - [createD3NumberFormatter tests](https://github.com/apache-superset/superset-ui/blob/master/packages/superset-ui-number-format/test/factories/createD3NumberFormatter.test.js)
   
   I also add backward compatibility shim in `setupFormatter.js` to point `+,` format, which is listed as one of default values in dropdown, but is now considered an invalid format by latest D3 to point to `+,d`, which is the correct equivalent format. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org