Skip to content

Commit aa90d47

Browse files
committed
Make middleware a required discriminator in router.map()
This eliminates ambiguity when route names like `action` (from the `form` helper) collide with the `action` property in middleware-wrapped actions. Previously, type guards checked for `action` or `actions` properties to determine if an object was middleware-wrapped. This caused confusion because `{ index: fn, action: fn }` could match `{ middleware?, action: fn }`. Now, `middleware` is the sole discriminator. Breaking changes: - `{ action: fn }` without `middleware` no longer works - use plain `fn` - `{ actions: {...} }` without `middleware` no longer works - use plain object
1 parent 6f4ec7b commit aa90d47

5 files changed

Lines changed: 176 additions & 155 deletions

File tree

‎demos/bookstore/app/routes.ts‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { route, form, resources } from '@remix-run/fetch-router'
1+
import { del, get, post, put, route, form, resources } from '@remix-run/fetch-router'
22

33
export let routes = route({
44
assets: '/assets/*path',
@@ -7,10 +7,7 @@ export let routes = route({
77
// Simple static routes
88
home: '/',
99
about: '/about',
10-
contact: route('/contact', {
11-
index: { method: 'GET', pattern: '/' },
12-
action: { method: 'POST', pattern: '/' },
13-
}),
10+
contact: form('contact'),
1411
search: '/search',
1512

1613
// Fragments
@@ -29,13 +26,13 @@ export let routes = route({
2926
auth: {
3027
login: form('login'),
3128
register: form('register'),
32-
logout: { method: 'POST', pattern: '/logout' },
29+
logout: post('logout'),
3330
forgotPassword: form('forgot-password'),
3431
resetPassword: form('reset-password/:token'),
3532
},
3633

3734
// Account section (protected, nested routes)
38-
account: route('/account', {
35+
account: route('account', {
3936
index: '/',
4037
settings: form('settings', {
4138
formMethod: 'PUT',
@@ -52,27 +49,27 @@ export let routes = route({
5249
}),
5350

5451
// Cart and shopping
55-
cart: route('/cart', {
56-
index: { method: 'GET', pattern: '/' },
52+
cart: route('cart', {
53+
index: get('/'),
5754

5855
// API-style endpoints under /cart/api
5956
api: {
60-
add: { method: 'POST', pattern: '/api/add' },
61-
update: { method: 'PUT', pattern: '/api/update' },
62-
remove: { method: 'DELETE', pattern: '/api/remove' },
57+
add: post('/api/add'),
58+
update: put('/api/update'),
59+
remove: del('/api/remove'),
6360
},
6461
}),
6562

6663
// Checkout flow
67-
checkout: route('/checkout', {
68-
index: { method: 'GET', pattern: '/' },
69-
action: { method: 'POST', pattern: '/' },
70-
confirmation: { method: 'GET', pattern: '/:orderId/confirmation' },
64+
checkout: route('checkout', {
65+
index: get('/'),
66+
action: post('/'),
67+
confirmation: get('/:orderId/confirmation'),
7168
}),
7269

7370
// Admin section (protected, showcases full CRUD on multiple resources)
74-
admin: route('/admin', {
75-
index: { method: 'GET', pattern: '/' },
71+
admin: route('admin', {
72+
index: get('/'),
7673

7774
// Full CRUD on books
7875
books: resources('books', { param: 'bookId' }),

‎packages/fetch-router/CHANGELOG.md‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,48 @@ This is the changelog for [`fetch-router`](https://github.com/remix-run/remix/tr
9494
})
9595
```
9696

97+
- BREAKING CHANGE: The `middleware` property is now required (not optional) in controller and action objects that use the `{ middleware, actions }` or `{ middleware, action }` format. This eliminates ambiguity when route names like `action` collide with the `action` property name.
98+
99+
```tsx
100+
// Before: { action } without middleware was allowed
101+
router.map(routes.home, {
102+
action() {
103+
return new Response('Home')
104+
},
105+
})
106+
107+
// After: just use a plain request handler function instead
108+
router.map(routes.home, () => {
109+
return new Response('Home')
110+
})
111+
112+
// Before: { actions } without middleware was allowed
113+
router.map(routes, {
114+
actions: {
115+
home() {
116+
return new Response('Home')
117+
},
118+
},
119+
})
120+
121+
// After: just use a plain controller object instead
122+
router.map(routes, {
123+
home() {
124+
return new Response('Home')
125+
},
126+
})
127+
128+
// With middleware, the syntax remains the same (but middleware is now required)
129+
router.map(routes, {
130+
middleware: [auth()],
131+
actions: {
132+
home() {
133+
return new Response('Home')
134+
},
135+
},
136+
})
137+
```
138+
97139
## v0.12.0 (2025-11-25)
98140

99141
- BREAKING CHANGE: Moved all response helpers to `@remix-run/response`. Update your imports:

‎packages/fetch-router/src/lib/controller.ts‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,15 @@ export type Controller<routes extends RouteMap> =
1111
| ControllerWithoutMiddleware<routes>
1212

1313
type ControllerWithMiddleware<routes extends RouteMap> = {
14-
middleware?: Middleware[]
14+
middleware: Middleware[]
1515
actions: ControllerWithoutMiddleware<routes>
1616
} & (routes extends Record<string, any>
1717
? {
18-
// Explicitly exclude route name keys from objects with `actions`
18+
// Explicitly exclude route name keys from objects with `middleware`
1919
[name in keyof routes as routes extends any ? never : name]?: never
2020
}
2121
: {})
2222

23-
export function hasActions<routes extends RouteMap>(
24-
controller: any,
25-
): controller is ControllerWithMiddleware<routes> {
26-
return typeof controller === 'object' && controller != null && 'actions' in controller
27-
}
28-
2923
// prettier-ignore
3024
type ControllerWithoutMiddleware<routes extends RouteMap> = routes extends any ?
3125
({
@@ -35,9 +29,8 @@ type ControllerWithoutMiddleware<routes extends RouteMap> = routes extends any ?
3529
never
3630
)
3731
} & {
38-
// Explicitly exclude `middleware` and `actions` from objects with route name keys
32+
// Explicitly exclude `middleware` from objects with route name keys
3933
middleware?: never
40-
actions?: never
4134
}) :
4235
never
4336

@@ -49,16 +42,10 @@ export type Action<method extends RequestMethod | 'ANY', pattern extends string>
4942
| RequestHandler<method, Params<pattern>>
5043

5144
type RequestHandlerWithMiddleware<method extends RequestMethod | 'ANY', pattern extends string> = {
52-
middleware?: Middleware<method, Params<pattern>>[]
45+
middleware: Middleware<method, Params<pattern>>[]
5346
action: RequestHandler<method, Params<pattern>>
5447
}
5548

56-
export function hasAction<method extends RequestMethod | 'ANY', pattern extends string>(
57-
action: any,
58-
): action is RequestHandlerWithMiddleware<method, pattern> {
59-
return typeof action === 'object' && action != null && 'action' in action
60-
}
61-
6249
/**
6350
* Build an `Action` type from a string, `RoutePattern`, or `Route`.
6451
*/
@@ -68,3 +55,33 @@ export type BuildAction<method extends RequestMethod | 'ANY', route extends stri
6855
route extends RoutePattern<infer pattern> ? Action<method, pattern> :
6956
route extends Route<infer _, infer pattern> ? Action<method, pattern> :
7057
never
58+
59+
/**
60+
* Runtime shape for a controller with middleware.
61+
*/
62+
export interface ControllerWithMiddlewareShape {
63+
middleware: Middleware[]
64+
actions: Record<string, unknown>
65+
}
66+
67+
/**
68+
* Check if an object has middleware and an `actions` property (controller with middleware).
69+
*/
70+
export function isControllerWithMiddleware(obj: unknown): obj is ControllerWithMiddlewareShape {
71+
return typeof obj === 'object' && obj != null && 'middleware' in obj && 'actions' in obj
72+
}
73+
74+
/**
75+
* Runtime shape for an action with middleware.
76+
*/
77+
export interface ActionWithMiddlewareShape {
78+
middleware: Middleware[]
79+
action: RequestHandler<any, any>
80+
}
81+
82+
/**
83+
* Check if an object has middleware and an `action` property (action with middleware).
84+
*/
85+
export function isActionWithMiddleware(obj: unknown): obj is ActionWithMiddlewareShape {
86+
return typeof obj === 'object' && obj != null && 'middleware' in obj && 'action' in obj
87+
}

‎packages/fetch-router/src/lib/router.test.ts‎

Lines changed: 11 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -177,24 +177,6 @@ describe('router.map()', () => {
177177
assert.equal(await response.text(), 'Home')
178178
})
179179

180-
it('maps a single route to an action without middleware', async () => {
181-
let routes = route({
182-
home: '/',
183-
})
184-
185-
let router = createRouter()
186-
187-
router.map(routes.home, {
188-
action() {
189-
return new Response('Home')
190-
},
191-
})
192-
193-
let response = await router.fetch('https://remix.run')
194-
assert.equal(response.status, 200)
195-
assert.equal(await response.text(), 'Home')
196-
})
197-
198180
it('maps a single route to an action with middleware', async () => {
199181
let routes = route({
200182
home: '/',
@@ -269,34 +251,6 @@ describe('router.map()', () => {
269251
assert.equal(await response.text(), 'Blog Post 1')
270252
})
271253

272-
it('maps a route map to actions without middleware', async () => {
273-
let routes = route({
274-
home: '/',
275-
blog: '/blog',
276-
})
277-
278-
let router = createRouter()
279-
280-
router.map(routes, {
281-
actions: {
282-
home() {
283-
return new Response('Home')
284-
},
285-
blog() {
286-
return new Response('Blog')
287-
},
288-
},
289-
})
290-
291-
let response = await router.fetch('https://remix.run')
292-
assert.equal(response.status, 200)
293-
assert.equal(await response.text(), 'Home')
294-
295-
response = await router.fetch('https://remix.run/blog')
296-
assert.equal(response.status, 200)
297-
assert.equal(await response.text(), 'Blog')
298-
})
299-
300254
it('maps a route map to actions with middleware', async () => {
301255
let routes = route({
302256
home: '/',
@@ -333,13 +287,17 @@ describe('router.map()', () => {
333287

334288
let router = createRouter()
335289

336-
router.map(routes, {
337-
middleware: [],
338-
// @ts-expect-error - should not allow middleware alongside actions
339-
home() {
340-
return new Response('OK')
341-
},
342-
})
290+
// This is a compile-time type check only - we use a never-executed block
291+
// to verify that TypeScript catches the error without running the code.
292+
if (false as boolean) {
293+
router.map(routes, {
294+
middleware: [],
295+
// @ts-expect-error - should not allow middleware alongside actions
296+
home() {
297+
return new Response('OK')
298+
},
299+
})
300+
}
343301
})
344302

345303
it('supports middleware in nested controllers', async () => {
@@ -494,19 +452,6 @@ describe('router.get()', () => {
494452
assert.equal(await response.text(), 'Home')
495453
})
496454

497-
it('maps a single route to an action object without middleware', async () => {
498-
let router = createRouter()
499-
router.get('/', {
500-
action() {
501-
return new Response('Home')
502-
},
503-
})
504-
505-
let response = await router.fetch('https://remix.run')
506-
assert.equal(response.status, 200)
507-
assert.equal(await response.text(), 'Home')
508-
})
509-
510455
it('maps a single route to an action with middleware', async () => {
511456
let requestLog: string[] = []
512457
let router = createRouter()

0 commit comments

Comments
 (0)