Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript readonly data array is incompatible with data property interface definition #2309

Closed
mward-sudo opened this issue Jun 18, 2022 · 2 comments · Fixed by #2313
Closed

Comments

@mward-sudo
Copy link

The Problem

I'm attempting to use a functional programming style to the extent it is possible to do so using the JavaScript frameworks I am using.

As part of this effort, I am defining my arguments and data constructs as readonly using TypeScript so that my data is not mutable.

However, I am experiencing an issue with Victory in that the TypeScript interface for the data property (on the VictoryLine component for example) cannot accept a readonly value because its typescript definition is using the mutable any type.

I get the following error:

The type 'ChartData' is 'readonly' and cannot be assigned to the mutable type 'any[]'.

Reproduction

https://codesandbox.io/s/winter-tree-00wu4z?file=/index.tsx

Potential Solution

In limited testing I tried changing the definition for the data property from any[] to any[] | readonly any[] and this solved the TypeScript error, but I haven't tested this properly.

If this issue is accepted, and the above is deemed an acceptable solution, then I will create a PR that modifies the TypeScript interfaces in the correct places.

@becca-bailey
Copy link
Contributor

Thanks for reporting this issue! We have been working on a Typescript conversion on our end, and there might be some kinks to work out with some of the typings. FYI @scottrippey.

@scottrippey
Copy link
Member

scottrippey commented Jun 20, 2022

I see no issue with marking data as readonly, since we never mutate it. PR #2313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants