fix bug (typescript) in creating log scales

[tkonopka]

Fixes a typescript-related bug in a function that creates log scales (package @nivo/scales)

Edited: adjusts usage of scale-related ts types in packages @nivo/bar, @nivo/marimekko, @nivo/scatterplot, @nivo/stream

Context

The function createLogScale creates an object correctly. However, the way the object is returned at the end creates confusion for the TypeScript compiler, making it believe the object has an incorrect type.

As an example, consider:

// prep
const mySpec: ScaleLogSpec = { type: 'log', min: 1, max: 1000 }
const myData = { all: [1, 1000], min: 1, max: 1000 }
// create scale objects
const myScale1: Scale<number, number> = createLogScale(mySpec, myData, 500, 'x') // gives error
const myScale2: Scale<any, any> = computeScale(mySpec, myData, 500, 'x')         // gives error  

Currently, the compiler gives errors, stating that outputs from createLogScale and from computeScale are incompatible with the types on the left-hand-side. The outputs actually carry all the right content - its just that the compiler is confused. The fix gives typescript a clearer indication about the returned object.

I tried to create unit tests that would yield an error before the fix and pass after the fix, but I couldn't manage. The reason is that this is problem appears only during builds of code like above. Putting such code in unit tests shows messages in an IDE, but executes without problems, regardless of the fix. To trigger an error, such code should be in the package src. However, @nivo/scales doesn't have/need such code in src. That pattern is found in other packages.

Impact

The issue addressed by this fix is the likely cause why some nivo packages occasionally use castings like xScale as any. With this edit, such fragments could be replaced by xScale as AnyScale or even plain xScale.

Affected packages are Bar, Marimekko, ScatterPlot, Stream. It could be relevant to Line once that is migrated to TypeScript (#1932). Swarmplot could be extended to support log scales, so this would be relevant there too.

I can edit those packages and remove ~24 linting warnings. If so, changes would be dispersed across several packages. What do you think?

Edited:

After discussion, scope expanded to update usage of typescript types for scale objects across packages.

• @nivo/scatterplot

  • removed "as any" in the context of scales

• @nivo/bar

  • removed "as any" in the context of scales
  • added types to layer props. The Bar and BarCanvas components had comments conveying layer props were set to "any" because of issues with scales. I added an explicit type in both cases. There were some conflicts between the layer props being created in Bar and BarCanvas and the draft type definitions in type.ts. In those cases, I opted to include more fields in the layer props (not removing anything). Existing code that relies on these props should work as before.

• @nivo/marimekko

  • refactored creation of scales to use @nivo/scales rather than d3-scale. This is just so that scale objects conform to nivo types so that they can be passed to Axes and Grid.
  • updated package dependencies to add @nivo/scales and remove d3-scale
  • removed "as any" in the context of scales
  • added some simple tests

• @nivo/stream

  • refactored creation of scales to use @nivo/scales rather than d3-scale. Again, this is just so that objects conform to nivo types so that they can be passed to Axes and Grid.
  • updated package dependencies to remove d3-scale (@nivo/scales already present)
  • removed "as any" in the context of scales

Some of the changes involve replacing Scale<any, any> by AnyScale. The latter is defined in @nivo/scales and it is actually the same as Scale<any, any>. The advantage is that AnyScale can now be restricted, if needed, in only one place.

Overall, the changes do not add external features and there should be no breaking changes. The number of warnings emited from make packages-lint decreased from 287 to 254.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b07d3c9:

Sandbox	Source
nivo	Configuration

[plouc]

That's a good catch, how I could have missed that :(, I have no objection updating all packages, on the contrary, those any are far from ideal 😅, so please go ahead if you feel like it.

I also wanted to thank you for all your great contributions, also helping others with issues, that's very helpful!

[tkonopka]

Thank you for sharing all this work. Happy to add a drop to it.

I'll make the edits to the packages affected.

[plouc]

LGTM, thank you for fixing the typings on the bar package and adding some tests for the marimekko.