Skip to content

Commit 078e1c9

Browse files
authored
fix: _middleware.ts supports Optional Parameter and Regexp routing (#332)
1 parent 59df066 commit 078e1c9

File tree

3 files changed

+91
-46
lines changed

3 files changed

+91
-46
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { createMiddleware } from 'hono/factory'
2+
import { createRoute } from '../../../src/factory'
3+
4+
const addHeader = createMiddleware(async (c, next) => {
5+
await next()
6+
c.res.headers.set('x-message', 'from middleware')
7+
})
8+
9+
export default createRoute(addHeader)

src/server/server.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ export const createApp = <E extends Env>(options: BaseServerOptions<E>): Hono<E>
181181

182182
// Apply extra middleware for parent routing patterns like /:lang{en} or /:lang?
183183
const rootPath = dir.replace(rootRegExp, '')
184-
const isRootLevel = !rootPath.includes('/') || rootPath === ''
184+
const isRootLevel = !rootPath.includes('/')
185185
const isSimpleStructure = !Object.keys(content).some((f) => f.includes('/'))
186186

187187
if (Object.keys(content).length > 0 && isRootLevel && isSimpleStructure) {
@@ -231,7 +231,8 @@ export const createApp = <E extends Env>(options: BaseServerOptions<E>): Hono<E>
231231
const shouldApply = middlewareDir === dir || dir.startsWith(middlewareDir + '/')
232232

233233
if (middleware.default && shouldApply) {
234-
subApp.use(...middleware.default)
234+
// Use a dynamic route pattern that matches all paths including root
235+
subApp.use('/:*{.+}?', ...middleware.default)
235236

236237
// Track that this middleware has been applied to this directory
237238
if (!appliedMiddlewaresByDirectory.has(dir)) {

test-integration/apps.test.ts

Lines changed: 79 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,70 +1057,105 @@ describe('Renderer loading when mounted with special route patterns', () => {
10571057
eager: true,
10581058
})
10591059

1060+
const MIDDLEWARE = import.meta.glob(
1061+
'../mocks/app-with-mount-patterns/routes/**/_middleware.(tsx|ts)',
1062+
{
1063+
eager: true,
1064+
}
1065+
)
1066+
10601067
describe('RegExp pattern mounting (/:lang{en})', () => {
10611068
// Reproduce the scenario where parent app uses RegExp routing
10621069
const app = createApp({
10631070
root: '../mocks/app-with-mount-patterns/routes',
10641071
ROUTES: ROUTES as any,
10651072
RENDERER: RENDERER as any,
1073+
MIDDLEWARE: MIDDLEWARE as any,
10661074
})
10671075

1068-
const apps = new Hono()
1069-
1070-
// This is the problematic setup with RegExp pattern
1071-
apps.route('/:lang{en}', app)
1072-
apps.route('/', app) // Additional route as mentioned in the original issue
1073-
1074-
it('Should return rendered index.tsx with _renderer.tsx', async () => {
1075-
// Test accessing /en (should show both index.tsx and _renderer.tsx)
1076-
const res = await apps.request('/en')
1077-
const html = await res.text()
1078-
1079-
expect(res.status).toBe(200)
1080-
expect(html).toContain('index.tsx')
1081-
expect(html).toContain('_renderer.tsx')
1082-
})
1083-
1084-
it('Should return rendered foo.tsx with _renderer.tsx', async () => {
1085-
// Test accessing /en/foo
1086-
const res = await apps.request('/en/foo')
1087-
const html = await res.text()
1076+
const routeAndPaths = [
1077+
{
1078+
route: '/:lang{en}',
1079+
paths: ['/en', '/en/foo'],
1080+
},
1081+
{
1082+
route: '/bar/:lang{en}',
1083+
paths: ['/bar/en', '/bar/en/foo'],
1084+
},
1085+
{
1086+
route: '/bar/buzz/:lang{en}',
1087+
paths: ['/bar/buzz/en', '/bar/buzz/en/foo'],
1088+
},
1089+
]
10881090

1089-
expect(res.status).toBe(200)
1090-
expect(html).toContain('foo.tsx')
1091-
expect(html).toContain('_renderer.tsx')
1091+
routeAndPaths.forEach(({ route, paths }) => {
1092+
describe(route, () => {
1093+
const apps = new Hono()
1094+
// This is the problematic setup with RegExp pattern
1095+
apps.route(route, app)
1096+
apps.route('/', app) // Additional route as mentioned in the original issue
1097+
1098+
paths.forEach((path) => {
1099+
it(path, async () => {
1100+
const res = await apps.request(path)
1101+
const html = await res.text()
1102+
expect(res.status).toBe(200)
1103+
1104+
// Check which component should be rendered based on the path
1105+
const expectedComponent = path.endsWith('/foo') ? 'foo.tsx' : 'index.tsx'
1106+
expect(html).toContain(expectedComponent)
1107+
expect(html).toContain('_renderer.tsx')
1108+
expect(res.headers.get('x-message')).toBe('from middleware')
1109+
})
1110+
})
1111+
})
10921112
})
10931113
})
10941114

1095-
describe('Optional parameter mounting (/:lang?)', () => {
1115+
describe('Optional parameter mounting', () => {
10961116
// Reproduce the scenario where parent app uses optional parameter routing
10971117
const app = createApp({
10981118
root: '../mocks/app-with-mount-patterns/routes',
10991119
ROUTES: ROUTES as any,
11001120
RENDERER: RENDERER as any,
1121+
MIDDLEWARE: MIDDLEWARE as any,
11011122
})
11021123

1103-
const apps = new Hono()
1104-
1105-
// This is the problematic setup with optional parameter
1106-
apps.route('/:lang?', app)
1107-
1108-
it('Should return rendered content with _renderer.tsx when using optional parameters', async () => {
1109-
// Test accessing /en (should show both index.tsx and _renderer.tsx)
1110-
const res = await apps.request('/en')
1111-
const html = await res.text()
1112-
expect(res.status).toBe(200)
1113-
expect(html).toContain('index.tsx')
1114-
expect(html).toContain('_renderer.tsx')
1115-
})
1124+
const routeAndPaths = [
1125+
{
1126+
route: '/:lang?',
1127+
paths: ['/en', '/en/foo'],
1128+
},
1129+
{
1130+
route: '/bar/:lang?',
1131+
paths: ['/bar/en', '/bar/en/foo'],
1132+
},
1133+
{
1134+
route: '/bar/buzz/:lang?',
1135+
paths: ['/bar/buzz/en', '/bar/buzz/en/foo'],
1136+
},
1137+
]
11161138

1117-
it('Should return rendered foo.tsx with _renderer.tsx when using optional parameters', async () => {
1118-
// Test accessing /en/foo
1119-
const res = await apps.request('/en/foo')
1120-
const html = await res.text()
1121-
expect(res.status).toBe(200)
1122-
expect(html).toContain('foo.tsx')
1123-
expect(html).toContain('_renderer.tsx')
1139+
routeAndPaths.forEach(({ route, paths }) => {
1140+
describe(route, () => {
1141+
const apps = new Hono()
1142+
// This is the problematic setup with optional parameter
1143+
apps.route(route, app)
1144+
1145+
paths.forEach((path) => {
1146+
it(path, async () => {
1147+
const res = await apps.request(path)
1148+
const html = await res.text()
1149+
expect(res.status).toBe(200)
1150+
1151+
// Check which component should be rendered based on the path
1152+
const expectedComponent = path.endsWith('/foo') ? 'foo.tsx' : 'index.tsx'
1153+
expect(html).toContain(expectedComponent)
1154+
expect(html).toContain('_renderer.tsx')
1155+
expect(res.headers.get('x-message')).toBe('from middleware')
1156+
})
1157+
})
1158+
})
11241159
})
11251160
})
11261161
})

0 commit comments

Comments
 (0)