Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/nice-donuts-obey.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

fix(react-router): run action handlers for routes with middleware even if no loader is present
65 changes: 65 additions & 0 deletions packages/react-router/__tests__/router/context-middleware-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,71 @@ describe("context/middleware", () => {
]);
});

it("runs middleware even if no loader exists but an action is present", async () => {
let snapshot;
router = createRouter({
history: createMemoryHistory(),
routes: [
{
path: "/",
},
{
id: "parent",
path: "/parent",
middleware: [
async ({ context }, next) => {
await next();
// Grab a snapshot at the end of the upwards middleware chain
snapshot = context.get(orderContext);
},
getOrderMiddleware(orderContext, "a"),
getOrderMiddleware(orderContext, "b"),
],
children: [
{
id: "child",
path: "child",
middleware: [
getOrderMiddleware(orderContext, "c"),
getOrderMiddleware(orderContext, "d"),
],
action({ context }) {
context.get(orderContext).push("child action");
},
},
],
},
],
});

await router.navigate("/parent/child", {
formMethod: "post",
formData: createFormData({}),
});

expect(snapshot).toEqual([
// Action
"a middleware - before next()",
"b middleware - before next()",
"c middleware - before next()",
"d middleware - before next()",
"child action",
"d middleware - after next()",
"c middleware - after next()",
"b middleware - after next()",
"a middleware - after next()",
// Revalidation
"a middleware - before next()",
"b middleware - before next()",
"c middleware - before next()",
"d middleware - before next()",
"d middleware - after next()",
"c middleware - after next()",
"b middleware - after next()",
"a middleware - after next()",
]);
});

it("returns result of middleware in client side routers", async () => {
let values: unknown[] = [];
let consoleSpy = jest
Expand Down
6 changes: 3 additions & 3 deletions packages/react-router/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5744,7 +5744,7 @@ function getDataStrategyMatch(
return shouldRevalidateLoader(match, unstable_shouldRevalidateArgs);
},
resolve(handlerOverride) {
let { lazy, loader, middleware } = match.route;
let { lazy, loader, action, middleware } = match.route;

let callHandler =
isUsingNewApi ||
Expand All @@ -5754,10 +5754,10 @@ function getDataStrategyMatch(
(lazy || loader));

// If this match was marked `shouldLoad` due to a middleware and it
// doesn't have a `loader` to run and no `lazy` to add one, then we can
// doesn't have a `loader` or `action` to run and no `lazy` to add one, then we can
// just return undefined from the "loader" here
let isMiddlewareOnlyRoute =
middleware && middleware.length > 0 && !loader && !lazy;
middleware && middleware.length > 0 && !loader && !action && !lazy;

if (callHandler && !isMiddlewareOnlyRoute) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (callHandler && !isMiddlewareOnlyRoute) {
if (callHandler && (isMutationMethod(request.method) || !isMiddlewareOnlyRoute)) {

return callLoaderOrAction({
Expand Down