Add @superset-ui/time-format

[kristw]

🏆 Enhancements

Add @superset-ui/time-format, a module for formatting time. Functions getTimeFormatter and formatTime should be used instead of calling d3.format directly.

import { getTimeFormatter } from '@superset-ui/time-format';
const formatter = getTimeFormatter('%Y-%m-d');
console.log(formatter(new Date()));

or

import { formatTime } from '@superset-ui/time-format';
console.log(formatTime('%Y-%m-d', new Date()));

It is powered by a registry to support registration of custom formatting, with fallback to d3.utcFormat or d3.timeFormat (if the formatId starts with local!)

import { getTimeFormatterRegistry, formatTime, TimeFormatter } from '@superset-ui/time-format';

getTimeFormatterRegistry().registerValue('my_format', new TimeFormatter({
  id: 'my_format',
  formatFunc: v => `my special format of ${utcFormat('%Y')(v)}`
});

console.log(formatTime('my_format', new Date(2018)));
// prints 'my special format of 2018'

It also define constants for common d3 time formats. See TimeFormats.js.

import { TimeFormats } from '@superset-ui/time-format';

TimeFormats.DATABASE_DATETIME // '%Y-%m-%d %H:%M:%S'
TimeFormats.US_DATE // '%m/%d/%Y'

[codecov]

Codecov Report

Merging #38 into master will increase coverage by 0.17%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
+ Coverage   99.11%   99.28%   +0.17%     
==========================================
  Files          48       57       +9     
  Lines         450      560     +110     
  Branches       39       39              
==========================================
+ Hits          446      556     +110     
  Misses          2        2              
  Partials        2        2

Impacted Files	Coverage Δ
...kages/superset-ui-time-format/src/TimeFormatter.js	100% <100%> (ø)
...perset-ui-time-format/src/TimeFormatterRegistry.js	100% <100%> (ø)
...-time-format/src/factories/createMultiFormatter.js	100% <100%> (ø)
...-ui-time-format/src/formatters/smartDateVerbose.js	100% <100%> (ø)
...-time-format/src/TimeFormatterRegistrySingleton.js	100% <100%> (ø)
...time-format/src/factories/createD3TimeFormatter.js	100% <100%> (ø)
...ackages/superset-ui-time-format/src/TimeFormats.js	100% <100%> (ø)
...uperset-ui-time-format/src/formatters/smartDate.js	100% <100%> (ø)
packages/superset-ui-time-format/src/utils.js	100% <100%> (ø)
... and 8 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 431b7b2...2893ec3. Read the comment docs.

[kristw]

Just made updates and this one should be ready to review.

[williaster]

overall lgtm! had a couple minor comments.

[williaster]

what do you think about having the user pass useLocalTime directly as a parameter instead of encoding in the format string? It seems a little more clear api-wise and would simplify splicing local! out, but also makes the format(format, value) signature become more complex.

given those tradeoffs I'd be okay with either, just thought I'd mention it. also not sure if Superset already uses !local and in any case it would probably be pretty straight forward to refactor in the future, too, if it was an issue. If we do keep !local, could we add a test for it?

[kristw]

There are factories which will take useLocalTime directly as parameter, which is the underlying way to construct a formatter.

I intended for the interface of the registry's format to be closest to current superset and provide backwards compatibility to how d3FormatString is stored as a single string in the database. If we introduce useLocalTime as another field then we need a separate control and another field to store in db.

[williaster]

ah I didn't realize this complexity, makes sense 👍