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

Argument of type 'readonly string[]' is not assignable to parameter of type 'string[]'. #22

Closed
nordowl opened this issue Nov 9, 2024 · 7 comments · Fixed by #23
Closed
Assignees

Comments

@nordowl
Copy link
Contributor

nordowl commented Nov 9, 2024

Describe the bug
I want to pass an urlState property of type string[] to a function. But that produces a TS error, because urlState uses readonly properties.

To Reproduce
Just some pseudo code

import { useUrlState } from "state-in-url"

function assignCats(cats: string[], owner: string) {
    return
}

type DefaultUrlState = {
    cats: string[]
    owner: string
}

const defaultState: DefaultUrlState = { cats: [], owner: "" }

const { setUrl, urlState } = useUrlState({ defaultState })

assignCats(urlState.cats, urlState.owner)

Trying to pass urlState.cats as the cats parameter throws:

Argument of type 'readonly string[]' is not assignable to parameter of type 'string[]'.
  The type 'readonly string[]' is 'readonly' and cannot be assigned to the mutable type 'string[]'

Expected behavior
No errors.

@asmyshlyaev177 asmyshlyaev177 self-assigned this Nov 9, 2024
@asmyshlyaev177
Copy link
Owner

asmyshlyaev177 commented Nov 9, 2024

Can change assignCats to

function assignCats(cats: string[] | readonly string[], owner: string) {
    return
}

urlState.cats will be readonly so you can't accidentally mutate it.

Possible to remove readonly from returned type if it problematic, can consider it.

@asmyshlyaev177
Copy link
Owner

@nordowl does it make sense to return regular data without readonly?

@nordowl
Copy link
Contributor Author

nordowl commented Nov 9, 2024

I think having to specify readonly everywhere you want to access the data is a step not needed. Especially if you then want to modify the data - e.g. you can't use array.push() anymore. Sure, you could use the spread operator and create a new array or what not, but that might potentially be more memory-intensive.

For now what I've done is use type casting with as, e.g. myFunc(urlState.myArray as string[]). But that's not great I think.

Having mutable array types would make things a little easier in my opinion.

@asmyshlyaev177
Copy link
Owner

@nordowl I guess it's not really needed, will remove it in a new version.

@asmyshlyaev177
Copy link
Owner

@nordowl will be released when https://github.com/asmyshlyaev177/state-in-url/actions/runs/11758298425 pass, issues closing automatically, not sure why.

@nordowl
Copy link
Contributor Author

nordowl commented Nov 9, 2024

@asmyshlyaev177 Thanks, I appreciate it!

@asmyshlyaev177
Copy link
Owner

asmyshlyaev177 commented Nov 9, 2024

@asmyshlyaev177 Thanks, I appreciate it!

It's released few minutes ago.

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.

2 participants