[PoC] Parent data in child loader #11319
Conversation
|
|
Hi @steinybot, Welcome, and thank you for contributing to React Router! Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once. You may review the CLA and sign it by adding your name to contributors.yml. Once the CLA is signed, the If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run. Thanks! - The Remix team |
| descendents: MatchNode[] | ||
| } | ||
|
|
||
| function createMatcherForest(matches: AgnosticDataRouteMatch[]): MatchNode[] { |
There was a problem hiding this comment.
This is possibly overkill but I didn't want to assume that the matches were in any particular order or that matches were strictly limited to a single path (in graph theory terms) even though that probably is the case (although the found router allows for multiple paths which is actually quite nice in some cases so I'd leave that capability in here since there is pretty much zero cost to it).
|
Unfortunately, this breaks one of the fundamental design decisions of the library. We don't want request waterfalls, which this specifically creates (whether the user wants it or not). This is going to make things like RSC harder to integrate later on as well. Another reason for loader independence is that you can selectively reload parts of the tree without having to trigger reloads of ancestor components. Those ancestor reloads would then lead to descendant reloads, which means pretty much a full-page reload. That would make RSC a moot point. I'm not going to close this in case @brophdawg11 disagrees, but this doesn't seem likely to be considered. The Middleware proposal is the more likely direction here. |
|
I agree middleware is the eventual solution to this type of sequential access. However, we are close to landing a new Thank you for the investigation! I'll try to remember to add a unit test into the "use cases" section of the |
In my use case I am using relay with https://github.com/loop-payments/react-router-relay. In that case the loader for the parent has already fetched the data that the child needs so there is no extra network requests. All the dependent loader is doing is extracting the fragment ref from the parents data.
What do you mean? Nothing here forces a waterfall. With this you only use it when you specifically want your child loader to run after the parents. It is not intended to be used to chain network requests, although if you really want to then there is nothing stopping you. In saying that, if the parent loaded data that was immediately available (just like the example) then the child could depend on that and make a network request and there would not be a waterfall.
The ancestor loaders have already resolved, you would not reload them.
That's great. I haven't fully digested how that is going to work yet. So long as this doesn't force everything to be sequential. We still want loaders with no dependencies to be able to make network requests in parallel. In relay, while you often have a "single fetch" i.e. query, although you can have multiple (actually two is possibly the most common as you have one for the navigation which almost never reloads and then one for the page). It looks like the |
|
Ok yeah so I think I get it. The descendent route could say which ancestor route it depends on in its context. The dataStrategy could then use this to do a similar thing I did here in this PR. It would resolve the ancestor and then pass that result to the dependent loader via its context. Seems like it ought to work. |
|
@brophdawg11 could you maybe provide some insights how the new Would it be something like this maybe? const parentRoutePromises = new Map<string, Promise<any>>();
export function getParentRoutePromise(id: string) {
return parentRouterPromises.get(id);
}
function defaultDataStrategy(
opts: DataStrategyFunctionArgs
): ReturnType<DataStrategyFunction> {
return Promise.all(opts.matches.map((m) => {
const promise = m.resolve();
if (m.route.id) {
parentRouterPromises.set(m.route.id, promise);
}
return promise;
}));
}While I think that this API does indeed allow for userland implementation of such a feature, I still think that providing an official way to get the promise of parent routes would be a great addition to the RR api. PS: When can we expect data strategy to become available in a RR release? Would really like to try working with that. |
|
The simplest way is to just call you loaders sequentially and collect the data along the way and pass it to the children: function sequentialDataStrategy({ matches }) {
let results = [];
let loaderData = {};
for (let match of matches) {
let result = await match.resolve(async (handler) => {
let data = await handler(data);
data[match.route.id] = data;
return { type: "data", result: data };
});
results.push(result);
}
return results;
}Your approach would also presumably work if you wanted to kick them off in parallel but then await internally to trigger the waterfalls - you would want to create a new |
Fixes #9188
Here is a proof of concept for using parent data in a child loader. It still needs a bit of work but it at least proves that it can be done.
Other than the places where the parent data hasn't been hooked up, the main problem to solve (if it is even an problem) is that the child will wait for all of the deferred parent data to be resolved before the child loader runs.
Take a look at the
data-routerexample.