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

feat: add meta as argument for key filter #2161

Closed
wants to merge 3 commits into from

Conversation

huozhi
Copy link
Member

@huozhi huozhi commented Sep 14, 2022

If the keys are non string, using filter will need to serialize the original keys to string, and do regext matching or string compare, adding a new argument meta for it, including serializedKey: string property for the filter to let user easily match the key

keyFilter(key: Key, meta: { serializedKey: string })

So that you can match the original key, or use regext to match the meta.serializedKey

mutate((key) => Array.isArray(key) && key[0].startsWith('/api'))
mutate((_key, meta) => /\/api/.test(meta.serializedKey))

In the future we can extend the meta with more info to match the keys more flexibily

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 14, 2022

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 9d6086a:

Sandbox Source
SWR-Basic Configuration
SWR-States Configuration
SWR-Infinite Configuration
SWR-SSR Configuration

@huozhi huozhi force-pushed the mutate/add-serialized-key branch from 5a6fa0f to 5268d85 Compare September 17, 2022 13:00
@huozhi huozhi changed the title feat: add serialized key as argument for key filter feat: add meta as argument for key filter Sep 17, 2022
@huozhi huozhi marked this pull request as ready for review September 17, 2022 13:03
@huozhi huozhi requested a review from shuding as a code owner September 17, 2022 13:03
@shuding
Copy link
Member

shuding commented Sep 18, 2022

My 2 cents: let's keep this on hold (I'm sorry!). The serialization process (under the hood called "stable hash") actually doesn't guarantee the result. That's also why we still expose it as unstable_serialize.

So I'm wondering if matching the hashed string is a good idea. Your example is actually the case: key[0].startsWith('/api') can't be expressed as /\/api/.test(meta.serializedKey). It's apparently wrong for keys like ['hey', '/api']. OK then naturally you start to think about how to match the serialized string with a regex confidentiality? It turns out that you can't because you don't know how it was hashed.

Some more examples:

useSWR({ id: 'foo' })
// ' will become " after serialization
useSWR(['/api/1', opts])
// opts might have any arbitrary content inside

How do you match these using the serialized key? The only conclusion I had is to never match serialized keys but match the original key, although it's more complicated.

@huozhi
Copy link
Member Author

huozhi commented Sep 20, 2022

Make sense, let's close it for now! And encourage users to use the accurate matching or compare when mutate multiple keys while multiple shapes of keys existing.
cc @koba04 Maybe we could add this case to docs v2?

Encouraged ✔️

// matching array key
mutate((key) => key[0].startsWith('/api'), data)
// matching string key
mutate((key) => typeof key === 'string' && key.startsWith('/api'), data)

Discouraged ❌

// mutate all when key's type is unsure (array or string)
mutate((key: any) => /\/api/.test(key.toString()))

@huozhi huozhi closed this Sep 20, 2022
@koba04
Copy link
Collaborator

koba04 commented Sep 20, 2022

@huozhi
Yeah, that's great 👍 Let's add it to the v2 docs.
Should we check if the key is an array in the encouraged pattern?

- mutate((key) => key[0].startsWith('/api'), data)
+ mutate((key) => Array.isArray(key) && key[0].startsWith('/api'), data)

@huozhi
Copy link
Member Author

huozhi commented Sep 20, 2022

@koba04 Yeah but only if you're using array key. The first 2 cases I posted are both the ways to go

@huozhi huozhi deleted the mutate/add-serialized-key branch January 23, 2023 14:44
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 this pull request may close these issues.

3 participants