typescript(vx-legend): re-write package in TypeScript #551
Conversation
williaster
added
馃悰 bug
馃挜 breaking changes
馃 typescript
馃殌enhancement
and removed
馃挜 breaking changes
馃悰 bug
labels
| @@ -40,6 +40,8 @@ const thresholdScale = scaleThreshold({ | |||
| range: ['#f2f0f7', '#dadaeb', '#bcbddc', '#9e9ac8', '#756bb1', '#54278f'], | |||
| }); | |||
|
|
|||
| global.s = thresholdScale; | |||
There was a problem hiding this comment.
testing, will remove
| 'labelDelimiter' | 'labelLower' | 'labelUpper' | ||
| >): LabelFormatterFactory<Datum, Output, ScaleThreshold<Datum, Output>> { | ||
| return ({ scale, labelFormat }) => (d, i) => { | ||
| let [x0, x1] = scale.invertExtent(scale.range()[i]); |
There was a problem hiding this comment.
this is messy, the problem is that the previous implementation used scale.range() and passed that as the domain prop. that is no longer valid with types, and it's impossible to compute the right thresholds directly from the domain value d. the current implementation assumes that domain.length === range.length 馃憥
the domain of a threshold scale also is not guaranteed to be a number so we have extra checks for that here.
There was a problem hiding this comment.
after going through the d3 docs more, if domain has n items then the range of a valid scale should have n+1, so I think this is safe. I updated the logic to check that the item corresponding to index i is within the range, and added a comment. we can update in the future if need be.
| export type ScaleThreshold<Input extends BaseInput, Output> = d3Scale.ScaleThreshold<Input, Output>; | ||
| export type ScaleQuantile<Output> = d3Scale.ScaleQuantile<Output>; | ||
|
|
||
| // @TODO BaseInput only needed for `ScaleThreshold` |
There was a problem hiding this comment.
I'm also not sure how to get around this cleanly. only ScaleThreshold has restrictions on Input, but that vastly restricts uses of the others. @kristw do you have any advance TS ideas?
There was a problem hiding this comment.
might be able to use conditional types here
type ScaleType<Input, Output> = Input extends BaseInput
? ScaleThreshold<Input, Output> | NonThresholdType<Input, Output>
: NonThresholdType<Input, Output>;There was a problem hiding this comment.
I spent a lot of time on this and couldn't get the conditional typing to work 馃槺 I am TS-ignoring it for now and will sync with @kristw and others when they're back from holiday.
| ...restProps | ||
| }: LegendQuantileProps<Output>) { | ||
| // transform range into input values because it may contain more elements | ||
| const domain = inputDomain || scale.range().map(output => scale.invertExtent(output)[0]); |
There was a problem hiding this comment.
this similarly just passed scale.range() as domain previously, this works correctly though. the note about the range I don't think is necessarily always true.
williaster
force-pushed
the
chris--typescript-vx-legend-ii
branch
from
1c11bc2
to
aef49d3
Compare
williaster
changed the title
|
woot! finally 馃槄 |
馃殌 Enhancements
This PR builds off #488 which introduces
Typescriptbuild config, and re-writes the@vx/legendpackage in TypeScript.This one was a bit gnarly vs some. Closes #546.
馃挜 Breaking Changes
React.Fragments, which requires bumping thepeerDepfor react to^16.3.0-0indexis unchanged):src/legends/*=>src/*Tests
/legenddemo.d.tsfilesScreen shots
Prod
Local
@hshoff @kristw