Convert time-format to TypeScript

[kristw]

🏆 Enhancements

• Convert @superset-ui/time-format to TypeScript

💔 Breaking Changes

• BREAKING CHANGE: Make formatter always returns string. This is different from previous behavior.

Value	formatted value (before)	formatted value (after)
null	null	'null'
undefined	undefined	'undefined'
NaN	NaN	'NaN'

[codecov]

Codecov Report

Merging #78 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #78   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          68     68           
  Lines         754    774   +20     
  Branches      150    182   +32     
=====================================
+ Hits          754    774   +20

Impacted Files	Coverage Δ
...-time-format/src/TimeFormatterRegistrySingleton.ts	100% <ø> (ø)
...-ui-time-format/src/formatters/smartDateVerbose.ts	100% <ø> (ø)
...uperset-ui-time-format/src/formatters/smartDate.ts	100% <ø> (ø)
...ackages/superset-ui-time-format/src/TimeFormats.ts	100% <ø> (ø)
...kages/superset-ui-time-format/src/TimeFormatter.ts	100% <100%> (ø)
...perset-ui-time-format/src/TimeFormatterRegistry.ts	100% <100%> (ø)
...time-format/src/factories/createD3TimeFormatter.ts	100% <100%> (ø)
packages/superset-ui-time-format/src/utils.ts	100% <100%> (ø)
...-time-format/src/factories/createMultiFormatter.ts	100% <100%> (ø)
... and 1 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 c353610...41c27e3. Read the comment docs.

[mistercrunch]

Are we sure there are no truthy evaluation of what comes out of this function in the Superset codebase? Wouldn't it be safer to either return either null or a string, or an falsy empty string?

[kristw]

My understanding of what @mistercrunch means is when people do sth like

formatTime(time, 'formatString') || 'custom label when null or undefined'

This will work if the formatTime() function returns truthy/falsy. My rationale for making it string was to make the function complete in itself without relying on || 'fallback'. If dev want to manually handle labels for null or undefined then they can add stricter check before calling formatTime().

[williaster]

overall looks good to me. had one thought about .ts + .js test files vs @ts-ignore.

I also generally think that the coercion of null to a string is okay. I think it's messy/not strict enough to have instances of timeFormatter(blah) || 'fallback blah', devs should check blah themselves IMO. This could be easily changed to (blah && timeFormatter(blah)) || 'fallback'.

@xtinec might still have some thoughts.