Skip to content

Commit 0515049

Browse files
authored
fix: fixes middleware not being applied inside nested folder where the parent folder has no route (#311)
* test(middleware): add test to reproduce middleware not being applied inside nested folder where the parent folder has no route * fix: middleware not being applied to nested child route if parent does not have a route * chore: fix lint issues + formatting * refactor: make isMiddlewareAppliedToAncestor more readable * refactor: remove unnecessary relative path removal
1 parent b9b8975 commit 0515049

File tree

8 files changed

+72
-15
lines changed

8 files changed

+72
-15
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { createMiddleware } from 'hono/factory'
2+
3+
export default [
4+
createMiddleware(async (c, next) => {
5+
await next()
6+
c.header('grandparent-middleware-applied', 'true', { append: true })
7+
}),
8+
]
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default function IndexRoute() {
2+
return <h1>IndexRoute</h1>
3+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default function NamedRoute() {
2+
return <h1>Named Route</h1>
3+
}

src/client/runtime.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('buildCreateChildrenFn', () => {
88

99
const div = document.createElement('div')
1010
div.innerHTML = '<span>test</span><div>test2</div>'
11-
const result = await createChildren(div.childNodes)
11+
await createChildren(div.childNodes)
1212
expect(createElement).toHaveBeenNthCalledWith(1, 'SPAN', {
1313
children: ['test'],
1414
key: 1,

src/server/server.ts

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,9 @@ export const createApp = <E extends Env>(options: BaseServerOptions<E>): Hono<E>
6363
const app = options.app ?? new Hono()
6464
const trailingSlash = options.trailingSlash ?? false
6565

66-
// Track applied middleware to prevent duplication
67-
const appliedMiddleware = new Set<string>()
66+
// Track applied middleware per directory to prevent duplication while allowing inheritance
67+
// Key: directory path, Value: Set of middleware file paths applied to that directory
68+
const appliedMiddlewaresByDirectory = new Map<string, Set<string>>()
6869

6970
// Share context by AsyncLocalStorage
7071
app.use(async function ShareContext(c, next) {
@@ -165,16 +166,43 @@ export const createApp = <E extends Env>(options: BaseServerOptions<E>): Hono<E>
165166
new RegExp(escapeDir(directory) + '/_middleware.tsx?').test(x)
166167
)
167168

169+
// Helper function to check if middleware has been applied to any ancestor
170+
const isMiddlewareAppliedToAncestor = (
171+
middlewarePath: string,
172+
currentDir: string
173+
): boolean => {
174+
const dirParts = currentDir.split('/')
175+
const ancestorDirs: string[] = []
176+
for (let i = dirParts.length - 1; i > 0; i--) {
177+
ancestorDirs.push(dirParts.slice(0, i).join('/'))
178+
}
179+
return ancestorDirs.some((ancestorDir) =>
180+
appliedMiddlewaresByDirectory.get(ancestorDir)?.has(middlewarePath)
181+
)
182+
}
183+
168184
// Find middleware for current directory first
169185
let middlewareFile = findMiddleware(dir)
170186

171-
// Issue 290 fix: Search parent directories if no middleware found
187+
// Issue 290/310 fix: Search parent directories if no middleware found
172188
if (!middlewareFile) {
173189
const dirParts = dir.split('/')
174190
for (let i = dirParts.length - 1; i >= 0; i--) {
175191
const parentDir = dirParts.slice(0, i).join('/')
176-
middlewareFile = findMiddleware(parentDir)
177-
if (middlewareFile) break
192+
193+
// Don't go beyond the routes root directory
194+
if (!parentDir.includes(root)) {
195+
break
196+
}
197+
198+
const parentMiddleware = findMiddleware(parentDir)
199+
if (!parentMiddleware) continue
200+
201+
// Skip if this middleware is already applied to any ancestor
202+
if (isMiddlewareAppliedToAncestor(parentMiddleware, dir)) continue
203+
204+
middlewareFile = parentMiddleware
205+
break
178206
}
179207
}
180208

@@ -185,15 +213,16 @@ export const createApp = <E extends Env>(options: BaseServerOptions<E>): Hono<E>
185213
.replace('/_middleware.ts', '')
186214

187215
// Only apply if this directory should have this middleware
188-
const shouldApply =
189-
middlewareDir === dir ||
190-
(!appliedMiddleware.has(middlewareFile) && dir.startsWith(middlewareDir + '/'))
216+
const shouldApply = middlewareDir === dir || dir.startsWith(middlewareDir + '/')
191217

192218
if (middleware.default && shouldApply) {
193-
if (!appliedMiddleware.has(middlewareFile)) {
194-
appliedMiddleware.add(middlewareFile)
195-
}
196219
subApp.use(...middleware.default)
220+
221+
// Track that this middleware has been applied to this directory
222+
if (!appliedMiddlewaresByDirectory.has(dir)) {
223+
appliedMiddlewaresByDirectory.set(dir, new Set())
224+
}
225+
appliedMiddlewaresByDirectory.get(dir)?.add(middlewareFile)
197226
}
198227
}
199228

@@ -261,7 +290,7 @@ export const createApp = <E extends Env>(options: BaseServerOptions<E>): Hono<E>
261290

262291
let rootPath = getRootPath(dir)
263292
if (trailingSlash) {
264-
rootPath = /\/$/.test(rootPath) ? rootPath : rootPath + '/'
293+
rootPath = rootPath.endsWith('/') ? rootPath : rootPath + '/'
265294
}
266295
app.route(rootPath, subApp)
267296
}

src/server/utils/file.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export const filePathToPath = (filePath: string) => {
88
.replace(/\((.+?)\)/g, '')
99
.replace(/\[(.+?)\]/g, ':$1')
1010
.replace(/\/\/+/g, '/')
11-
return /^\//.test(filePath) ? filePath : '/' + filePath
11+
return filePath.startsWith('/') ? filePath : '/' + filePath
1212
}
1313

1414
/*

src/vite/components/honox-island.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ describe('HonoXIsland', () => {
99
componentExport: 'Test',
1010
Component: () => <div>Test</div>,
1111
props: {
12-
children: <TestComponent />,
12+
children: <TestComponent />,
1313
},
1414
})
1515
// XXX: tested by internal implementation

test-integration/apps.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,20 @@ describe('Nested Middleware', () => {
478478
expect(res.status).toBe(200)
479479
expect(res.headers.get('X-Admin-Middleware')).toEqual('true')
480480
})
481+
482+
it('Should apply grandparent middleware to child index route - /without-route/with-index', async () => {
483+
const res = await app.request('/without-route/with-index')
484+
expect(res.status).toBe(200)
485+
await expect(res.text()).resolves.toBe('<h1>IndexRoute</h1>')
486+
expect(res.headers.get('grandparent-middleware-applied')).toEqual('true')
487+
})
488+
489+
it('Should apply grandparent middleware to child named route - /without-route/with-named/named', async () => {
490+
const res = await app.request('/without-route/with-named/named')
491+
expect(res.status).toBe(200)
492+
await expect(res.text()).resolves.toBe('<h1>Named Route</h1>')
493+
expect(res.headers.get('grandparent-middleware-applied')).toEqual('true')
494+
})
481495
})
482496

483497
describe('<Script /> component', () => {

0 commit comments

Comments
 (0)