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

Possible key type mismatch #315

Open
mv-go opened this issue Oct 12, 2022 · 11 comments
Open

Possible key type mismatch #315

mv-go opened this issue Oct 12, 2022 · 11 comments

Comments

@mv-go
Copy link

mv-go commented Oct 12, 2022

According to docs something like this should be possible:
image

However, the following code triggers a TS type mismatch warning

const maybeUndefined = ref<string>()

const { data, error, mutate } = useSWRV(
  () => maybeUndefined.value && `my unuque key ${maybeUndefined.value}`,
  async () => {
    const r = await axiosInstance.get(`myUrl/${maybeUndefined.value}`)
    return r
  },
)

Warning reads:

Argument of type '() => string | undefined' is not assignable to parameter of type 'IKey'.
  Type '() => string | undefined' is not assignable to type 'keyFunction'.ts(2345)

Upon further digging the issue seems to be in type declaration for keyFunction

export declare type keyType = string | any[] | null;
declare type keyFunction = () => keyType;
export declare type IKey = keyFunction | keyType;

It would seem that amending keyType to

export declare type keyType = string | any[] | null | undefined;

would fix the issue.

However I'm not sure if this doesn't break something down the line :)

@mv-go
Copy link
Author

mv-go commented Oct 12, 2022

ATM I'm working around this by explicitly returning null in the keyFunction where necessary, however it'd be great to be able to return null || undefined as documentation states

@adamdehaven
Copy link
Member

I think if you defined your original const differently it would pass the checks:

const maybeUndefined = ref<string>('')

// or as null without even declaring a type
const maybeUndefined = ref(null)

But you're saying you should be able to define an "empty" ref, meaning it should be able to be undefined, which I think makes sense, because these should both eval to a falsy value:

const user = ref()
const { data: projects } = useSWRV(() => user.value && '/api/projects?uid=' + user.value.id, fetch)

const user = ref<string>('')
const { data: projects } = useSWRV(() => user.value && '/api/projects?uid=' + user.value.id, fetch)

@adamdehaven
Copy link
Member

However I'm not sure if this doesn't break something down the line

I'm more concerned about breaking the interface for IKey which it looks like cannot evaluate to undefined

@mv-go
Copy link
Author

mv-go commented Oct 13, 2022

Yes, I agree - this could be a breaking change. Speaking of which, I would consider switching the behaviour from falsy to strictly null || undefined. Consider the following case:

const id = ref<number>() // undefined
const { data } = useSWRV(
  () => id.value && `/posts/${id.value}`,
  fetch
)
id.value = 0 // not undefined, but still falsy

In this case, the fetch would not be triggered, however a post (a user, an object) with an id = 0 is a perfectly valid case.

However, maybe in this case, leaving the developer to explicitly return null might be actually not that bad of an idea :)

@adamdehaven
Copy link
Member

n this case, the fetch would not be triggered, however a post (a user, an object) with an id = 0 is a perfectly valid case.

In JavaScript, 0 is considered a falsy value. Source

@mv-go
Copy link
Author

mv-go commented Oct 14, 2022

In JavaScript, 0 is considered a falsy value. Source

I understand, yes. So in my example above you would suggest that fetch should not be triggered when id is changed from undefined to 0?

@adamdehaven
Copy link
Member

So in my example above you would suggest that fetch should not be triggered when id is changed from undefined to 0?

Correct, because they both evaluate as falsy.

@mv-go
Copy link
Author

mv-go commented Oct 14, 2022

Is this a desired behaviour?

It would seem common to have an object on the backend with an id = 0

@adamdehaven
Copy link
Member

Instead of thinking about something with an id, think about an iterator.

If I have a starting value of an iterator as let i = 0 and increment in a loop or something when I have data and/or processing something, I'd only want the fetcher to run when the value of i is non-falsy (i.e. not 0).

I see your use-case; however, I would think if you know you're dealing with falsy values coming from your data structure you would use something you know that cannot equate to falsy as your dependent fetcher key.

@mv-go
Copy link
Author

mv-go commented Oct 14, 2022

Fair enough. Should we consider implementing changes regarding my initial message in this issue - to include undefined in typings. Or shall we just close this one as irrelevant? :)

@adamdehaven
Copy link
Member

I'm still wondering if adding undefined to keyType invalidates the IKey interface

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

No branches or pull requests

2 participants