swr
swr copied to clipboard
Initial implementation of `useSWRList`
Implemented in accordance with the RFC. Initial implementation is
mostly a copy/paste of useSWR
, then moved things into arrays so we
can handle the list aspect of this correctly. Plenty of work still to be done,
especially around the duplication between useSWRList
& useSWR
,
plus more tests need to be written, but this gives us a basis for discussion.
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 8ca9b806907092f8da30996840ccad9a23b89d2d:
Sandbox | Source |
---|---|
SWR-Basic | Configuration |
SWR-States | Configuration |
SWR-Infinite | Configuration |
SWR-SSR | Configuration |
Just saw #2018 as I was opening this, whose implementation seems a lot simpler. I think dealing with an array of data
(as per Proposal A) results in a simpler implementation. The "Array of SWRs" approach makes it difficult/impossible to use useSWR
under the hood the way #2018 does, because we're not just dealing with a single piece of data, but all of the associated pieces (mutate
, loading state, etc.).
There's definitely a ton of duplication, but I didn't want to start seriously refactoring useSWR
itself to extract more reusable bits without feedback because that will def result in a lot of churn. I also am not 100% clear around which of these functions need to be stable cuz there's a lot of disabling of exhaustive deps in useSWR
that I'm not fully familiar with.
cc @shuding @promer94
Also looking at #2006, which interestingly did some things similarly to the way I did so that's cool.
Totally agreed. It is also my biggest concern that we are not reusing the implementation of SWR's core. We should evaluate the difficulty of making the core support a list of keys.
Another reason is that we still need to return an array of SWR objects, instead of an array of data
. (cc @promer94)
We should evaluate the difficulty of making the core support a list of keys.
Funny enough, as I was doing this, my galaxy brain thought was that, instead of trying to implement useSWRList
as "useSWR
with an array of keys", we should do the reverse and implement useSWR
as "useSWRList
but only a single key".
In terms of landing this, since v2 is still in beta, I think we have a few options:
- Land this "as-is" (except with more tests), do some additional refactoring as follow-on PRs.
- Extract more helpers from core first, then refactor this PR to take advantage of them.
- Use this PR to do a bigger refactor of and land it all at once.
- Something else?
Thoughts? How should I proceed?
A few things I did like from @promer94's implementations:
- Returning
results
as a key on an object gives the API more flexibility to expand in the future. - Or even expand now, with a root
mutate
,isValidating
,isLoading
that applies to all of the keys. - Adding the
key
(and possiblyoriginKey
) to the returned object also seems useful.
I'm not so much a fan of the children
/ items
of useSWRAggregator
, which seemed unnecessary, and in general, I think the API of useSWRList
, providing the array of SWR objects, is generally more useful than just an array of data.
I think for all of the above approaches, I'm thinking implementing those 3 ideas would be good, so unless you have any objections, I'll include those changes once we have a direction.
Concerns
Returning an array of SWR objects could lead to bad performance.
If we have 1000 keys, the component which uses useSWRList will rerender like 1001 times.
This is why react-query’s useQueries
, IMO, is really a bad api design.
SWR always proivder greate performance for users. So i want to keep that.
Some Configuaration's semantics are not clear
{
// should onSuccess be called multiple times with individual result and key ?
// or just once with list result and array keys ?
onSuccess: (data, key) => {
}
// Similar problem for
onError,
onErrorRetry
onDiscard
}
{
// should we support fallback here ?
fallback
}
children
/ items
of useSWRAggregator
is useful because it will have much batter performance and user can even access the result array in individual item component and it saves a lot of boilerplates
FWIW, I agree with the performance concerns. I included that as a con of this approach in the original RFC.
However, I think the semantics would derive directly from the idea of it just being an "array of SWRs": all of those would be called per-item. Obviously, this exacerbates the performance issues, but I don't think it's confusing. fallback
is a little odd because it would probably have to be structured like fallbackData
rather than fallback
in the original. Alternatively, we could make fallback
an array as well.
One of the weird things about the children
/ items
approach is that, because an individual item is subscribing directly to its key, you could feasibly end up in a place where an individual item is rendered because its data is complete but the aggregated result is still loading and thus empty.
Relatedly, there isn't any way to invalidate/mutate
an individual key at the top level with useSWRAggregator
. If you have a large list & need to invalidate a subset of those keys in a bulk update, you probs have to pull in the top-level mutate
from useSWRConfig
to accomplish it. That said, I don't think this would be difficult to add to that PR and shouldn't change the performance characteristics.
So where are we with this PR? Do we need to revisit the RFC and change the API design? @shuding @promer94
Hi all! Revisiting this again. Would love to push this or some sort of multi-key implementation forward. How can we advance this?
any plan on crossing the finish line? look forward to using this feature 🙏
@0xbe1 I'm still happy to work on this, but without a clear direction on the API design, there's not much I can do.
@promer94 @shuding Anything I can do to push this forward? We really need an API around this on our project. We've been working with a janky userland implementation that I would love to get rid of and replace with a blessed implementation, but without a direction or clear sign this can get merged in some form, we're at a bit of a standstill.
@promer94 @shuding Another ping to say I'm still interested in pushing this forward.