๐บ๏ธ Loader + Action Context (Middleware) #9564
Replies: 34 comments 63 replies
-
From @jacob-ebey: I like this quite a bit. The parallels to React context are nice and should be easy to teach. Avoiding mutation of a a loadContext like object is also nice. |
Beta Was this translation helpful? Give feedback.
-
From @jacob-ebey Open Questions:
// could be utilitized for your app and not live in every test
let t = dataFunctionTestHelper(loader | action);
t.context.set(userContext, { id: "yay!" });
// options to call loader / action
// get
let res = await t.fetch("/route");
// post
let res = await t.fetch("/route", new FormData());
let res = await t.fetch(new Request("/route", {
method: "post",
body: new FormData(),
}));
|
Beta Was this translation helpful? Give feedback.
-
From @jacob-ebey Another note, I think this is a breaking change and would require a major release. loadContext is currently totally open for whatever you want, if we make it have a get and set method we are potentially breaking apps and are restricting what you can do with loadContext. |
Beta Was this translation helpful? Give feedback.
-
From @brophdawg11 The only thing I can think of from a prior-art standpoint is Vue Router's beforeEnter, since they also support nested routes. But they don't address data-fetching in the same way and there's no type of "context" for passing data down the hierarchy AFAIK. I like the proposal, and prefer the react-like approach - primarily due to the ease of providing inferred typings for the TS crowd. A few off the cuff questions: I assume beforeLoader will be subject to the exact same shouldRevalidate logic as the loader? |
Beta Was this translation helpful? Give feedback.
-
what about instead of |
Beta Was this translation helpful? Give feedback.
-
Related issue in RR that I think would be resolved with this functionality: #9324 |
Beta Was this translation helpful? Give feedback.
-
It'd be nice if we could determine the dependency graph of the loaders based on inputs / outputs so that we could optimally run things in parallel. E.g. if theres a page like Might be able to determine those dependencies between loaders in the compiler. ๐ถ Don't fetch JSON waterfalls ๐ถ |
Beta Was this translation helpful? Give feedback.
-
I like more the React approach over the Express-like approach, but I think it shouldn't be for loaders only, it should also include actions, this would allow me to define a middleware to protect nested route to only be accessible for authenticated users or users with a role, regardless of the HTTP method used to reach that route. |
Beta Was this translation helpful? Give feedback.
-
I think it'd be great if we could set/update response headers such as Set-Cookie in that new function. Will it be possible? |
Beta Was this translation helpful? Give feedback.
-
the context get/set functionality reminds me strongly of this library (cls-hooked), maybe it is something that can help you design this! will this before loader stuff work with client side transition in Remix? |
Beta Was this translation helpful? Give feedback.
-
This comment in vercel/micro is related: vercel/micro#8 (comment) I like the react approach more although I wonder if you could remove the Something like this: function createMiddleware(fn){
let weakMap = new WeakMap();
return function (ctx) {
if (weakMap.has(ctx)) return weakMap.get(ctx);
let promise = fn(ctx);
weakMap.set(ctx, promise);
return promise;
}
}
const getUser = createMiddleware(ctx => {
if (!token) throw new UnauthorizedErrors();
return user; // or whatever
});
function loader({ context }){
let user = await getUser(context);
return json({});
} You'd still need something like |
Beta Was this translation helpful? Give feedback.
-
Digging this proposal. Has there been consideration for how getting data from hooks will factor into this |
Beta Was this translation helpful? Give feedback.
-
The use case I'm imagining here revolves around setting anonymous user ids for a/b testing, feature flagging, and other frontend analytics. I suspect that the root route would need a Assuming I'm following this correctly, it seems like the root This gets to my question: would it be feasible to invoke |
Beta Was this translation helpful? Give feedback.
-
Are there any special considerations that need to be taken for middlware with the recent addition of stream'd responses in Remix (ex: using |
Beta Was this translation helpful? Give feedback.
-
@ryanflorence For example, if you had a resource route to generate a PDF file, you may not need/want the i18n You could of course exit early from the i18n |
Beta Was this translation helpful? Give feedback.
-
@ryanflorence regarding your first open question:
Have you considered
Or, if you have considered it, but deem it too risky for now (the TC39 proposal is still in Stage 0 and the other implementations are behind some kind of feature flags or are marked experimental), it's at least worth keeping it in mind for future iterations of the API. |
Beta Was this translation helpful? Give feedback.
-
Would Remix use this API on the client or server side? Or both? I'm still hoping for a stable Remix API that would allow me to dynamically load additional blocking data/styles/scripts alongside a route's module and loader-data. (ref remix-run/remix#3355). |
Beta Was this translation helpful? Give feedback.
-
Is there a version of this that could be created that adds the hooks, but not the mutable context API? I'm not a huge fan of the mutable API being baked into remix/rr itself. What about leaving this up to the reader to attach their stores to the context themselves? This would allow users to take advantage of longer lived caching if required, as well as use |
Beta Was this translation helpful? Give feedback.
-
What you would middleware composition look like with this new API? Recently stumbled upon this lib which takes a |
Beta Was this translation helpful? Give feedback.
-
With this API, would beforeLoader be able to return deferred promises that all downstream loaders could individually decide to await for? |
Beta Was this translation helpful? Give feedback.
-
I wish we could run certain functions "on our way in" and others "on our way out." For example, we can already run Today each loader/action must currently commit its own changes to the remote KV store, which increases load at both ends. (Edit: One question this idea raises is whether middleware cleanup should work like |
Beta Was this translation helpful? Give feedback.
-
I like this proposal, but I think it might miss the "afterLoader" middleware ? (or maybe it was planned for later) My use case: I want to wrap all my loaders and actions automatically in order to log the errors before forwarding them to the client. Currently I am doing this manually for each of them: export const loader = async ({ request }: LoaderArgs) => {
return errorHandler((request) => {
...
})
}) Maybe I missed a better way ? |
Beta Was this translation helpful? Give feedback.
-
I do like the approach of "lifecycles" for routes from :
But what about adding a "middleware" to multiple routes? I think something like the layout route but for middlewares might work as well |
Beta Was this translation helpful? Give feedback.
-
I am attempting to integrate React Router into an existing project that utilizes Redux for state management, and I find it necessary to have a
To give a very simple example for the second case, it might look something like this: const beforeLoader = async ({ context }) => {
// assign new store for next page
context.store = createStore();
};
const loader = async ({ context }) => {
const { store } = context;
await Promise.all([
store.dispatch(fetchTheme()),
store.dispatch(fetchArticle()),
store.dispatch(fetchUser()),
]);
return { store };
};
const Root = () => {
const { store } = useLoaderData();
return (<Provider store={store}><Root /></Provider>);
} In both scenarios, it is crucial to have the But I am not asking for a Redux-specific feature. rather, just explaining that |
Beta Was this translation helpful? Give feedback.
-
Can I just check that this proposal would mean that we could avoid having to inject the react-query client into our loaders like Dominik suggests here? https://tkdodo.eu/blog/react-query-meets-react-router#querifying-the-example. I'm assuming that the loader would have a context passed to it and when setting up the routes we can just add the QueryClient to that context. If that's the case, it unlocks a whole bunch of nice-to-haves (as far as I can tell) like being able to use lazy routes because you can just import the loader from the route module. Note that I'm asking in a react-router sense and not a remix one. |
Beta Was this translation helpful? Give feedback.
-
๐ Hey folks! Now that v2 is out, middleware/context are back on the radar and being tracked in the Roadmap via https://github.com/remix-run/remix/issues/7643 and remix-run/remix#7645 |
Beta Was this translation helpful? Give feedback.
-
This would be really, really nice. React Router 6 solves a very nice issue in being able to resolve data requests outside of React, avoiding the " We can make these even uglier if someone decides the best way to architect their app to avoid DRY concerns would be to use a React provider to hold rendering the application until the auth session call has completed, and additionally write to something like local storage to access the session outside of React. |
Beta Was this translation helpful? Give feedback.
-
Do we know if this RFC still going ahead? :) |
Beta Was this translation helpful? Give feedback.
-
This caveat from Remix that there is no root location where you can add logic that affects all loaders makes Remix difficult to scale. Having to repeat authentication logic all over the app is error-prone and counterintuitive. I use Remix at work and always am embarrassed when my back-end developers ask about this and I have to explain the unfortunate reality. |
Beta Was this translation helpful? Give feedback.
-
Is this not solved with dataStrategy? Documentation is lacking however... https://api.reactrouter.com/v7/interfaces/react_router.DataStrategyFunction.html |
Beta Was this translation helpful? Give feedback.
-
Background
There are some patterns in v5 that don't have a clear cognate in v6.
Patterns in v5
v6.4 loaders
With v6.4 loaders, there are no similar patterns, loaders just have to repeat the same code down the route hierarchy.
It's easy to miss one of these and in the case of Remix, accidentally expose a route to the public.
Remix
Remix users are begging for middleware for things like global redirects, protecting routes, and more. Even our own sites need it. This would serve as the base for that.
Passing data
There's no way to pass data from a parent loader to a child loader. You can use React context or
<Outlet context>
to share data while rendering, but that only works for components, not loaders.Proposal:
beforeLoader
It is proposed we add a
beforeLoader
hook to routes.The primary concern for us has always been that we want loaders to run in parallel to avoid waterfalls and therefore artificial performance issues.
This proposed API keeps most data loading parallelized, but adds the ability for apps to opt-in to serial data loading among route hierarchies when the need arises.
For example:
Protecting Routes
beforeLoader
functions will be called seriallybeforeLoaders
have been been called, all matchingloaders
will be called in parallel (no change)Sharing Data
Before loader can be used to make data available to a route's own loader as well as the loaders of its child routes.
I have two API proposals here: express-like, and react-like.
Express-like middleware
This API already exists in a small way inside of Remix with
getLoadContext
. Each loader/beforeLoader would receive acontext
object (like an expressreq
) that beforeLoaders can mutate.context
, so piggy back therecontext
typesloader
s is wrong, and will make the values available in other loaders depending on the async timing of all of them running parallelReact-like context API
Calling a handful of unknown loaders in a hierarchy is basically the same thing as calling a bunch of components in a hierarchy. We could use a context API similar to
react-broadcastReact.createContext
.React.createContext
context.set/get
Tradeoffs
Open Questions
I'm pretty sure we need the
context.set(userContext, val)
instead of the simpleruserContext.set
so that multiple requests in Remix won't be colliding.context.set
is ourReact.createElement()
. It's what gives us control of a single request's context, instead of a global value shared by all users. That said, maybe there's a way to get around this that I'm not thinking of?This complicates testing loaders in isolation. While
createLoaderContext
takes a default value, might be useful to provide a test helper? As we get into implementation it might become more clear what, if anything, we should provide here.Is there prior-art to look to? The weasel here is nested routes. It's one thing to have middleware in a web framework like express where a single route handles a single request. Our task is different. A single URL matches multiple nested routes, we're not just trying to share data from one middleware to the next, we're trying to share it with arbitrary set of other matched routes. I can't think of any prior-art besides React.context.
Details
beforeLoader
are called serially beforeloaders
loaders
should still be called in parallelcontext.get()
should throw if the value has never been setcontext.set(type, undefined)
should throwcontext.set(type, null)
and any other falsey values (exceptundefined
) should be allowedBeta Was this translation helpful? Give feedback.
All reactions