diff --git a/e2e/base-path.spec.ts b/e2e/base-path.spec.ts index 19feef8772..6a1e6fc0e9 100644 --- a/e2e/base-path.spec.ts +++ b/e2e/base-path.spec.ts @@ -44,6 +44,33 @@ test.describe(`base-path`, () => { text: 'hello', }, }); + + // get trailing slash + const resGetTrailing = await request.get(`${baseUrl}hello/`); + expect(resGetTrailing.ok()).toBe(true); + expect(await resGetTrailing.json()).toEqual({ + ok: true, + request: { + handler: 'GET', + method: 'GET', + pathname: '/hello/', + }, + }); + + // post trailing slash + const resPostTrailing = await request.post(`${baseUrl}hello/`, { + data: 'hello', + }); + expect(resPostTrailing.ok()).toBe(true); + expect(await resPostTrailing.json()).toEqual({ + ok: true, + request: { + handler: 'POST', + method: 'POST', + pathname: '/hello/', + text: 'hello', + }, + }); }); test('router', async ({ page }) => { @@ -52,16 +79,27 @@ test.describe(`base-path`, () => { await waitForHydration(page); // push - await page.getByText('dynamic-push').click(); + await page + .getByRole('button', { name: 'dynamic-push', exact: true }) + .click(); await page.waitForURL(`${baseUrl}dynamic`); await expect( page.getByRole('heading', { name: 'Dynamic page' }), ).toBeVisible(); + // push trailing slash + await page.goto(baseUrl); + await waitForHydration(page); + await page.getByRole('button', { name: 'dynamic-push-trailing' }).click(); + await page.waitForURL(`${baseUrl}dynamic/`); + await expect( + page.getByRole('heading', { name: 'Dynamic page' }), + ).toBeVisible(); + // replace await page.goto(baseUrl); await waitForHydration(page); - await page.getByText('dynamic-replace').click(); + await page.getByRole('button', { name: 'dynamic-replace' }).click(); await page.waitForURL(`${baseUrl}dynamic`); await expect( page.getByRole('heading', { name: 'Dynamic page' }), @@ -115,4 +153,9 @@ async function basicTest(page: Page, baseUrl: string) { await expect( page.getByRole('heading', { name: 'Static page' }), ).toBeVisible(); + + await page.goto(`${baseUrl}static/`); + await expect( + page.getByRole('heading', { name: 'Static page' }), + ).toBeVisible(); } diff --git a/e2e/fixtures/base-path/src/pages/_layout.tsx b/e2e/fixtures/base-path/src/pages/_layout.tsx index 8f646cc1b0..e98e631ce0 100644 --- a/e2e/fixtures/base-path/src/pages/_layout.tsx +++ b/e2e/fixtures/base-path/src/pages/_layout.tsx @@ -24,6 +24,9 @@ export default async function RootLayout({
  • dynamic-push
    • +
  • + dynamic-push-trailing +
  • dynamic-replace diff --git a/e2e/fixtures/router-client-no-404/src/components/route-state.tsx b/e2e/fixtures/router-client-no-404/src/components/route-state.tsx index c4f47454b8..7f359fc1d5 100644 --- a/e2e/fixtures/router-client-no-404/src/components/route-state.tsx +++ b/e2e/fixtures/router-client-no-404/src/components/route-state.tsx @@ -17,14 +17,25 @@ export function PushMissingButton() { const router = useRouter(); const push = router.push as unknown as (to: string) => Promise; return ( - + <> + + + ); } diff --git a/e2e/fixtures/router-client/src/components/route-state.tsx b/e2e/fixtures/router-client/src/components/route-state.tsx index e213d42253..bbcfae9d06 100644 --- a/e2e/fixtures/router-client/src/components/route-state.tsx +++ b/e2e/fixtures/router-client/src/components/route-state.tsx @@ -36,6 +36,15 @@ export function RouteState() { router.push hash missing

    +

    + +

    +

    +

    + + Go to static trailing + +

    ); } diff --git a/e2e/fs-router.spec.ts b/e2e/fs-router.spec.ts index 015bf0dfba..eee1e0d1a7 100644 --- a/e2e/fs-router.spec.ts +++ b/e2e/fs-router.spec.ts @@ -41,6 +41,11 @@ test.describe('fs-router', () => { await expect(page.getByRole('heading', { name: 'Foo' })).toBeVisible(); }); + test('foo with trailing slash', async ({ page }) => { + await page.goto(`http://localhost:${port}/foo/`); + await expect(page.getByRole('heading', { name: 'Foo' })).toBeVisible(); + }); + test('nested/foo', async ({ page }) => { // /nested/foo is defined as a staticPath of /nested/[id] which matches this layout await page.goto(`http://localhost:${port}/nested/foo`); @@ -56,6 +61,20 @@ test.describe('fs-router', () => { ).toBeVisible(); }); + test('nested/baz with trailing slash', async ({ page }) => { + await page.goto(`http://localhost:${port}/nested/baz/`); + await expect( + page.getByRole('heading', { name: 'Nested Layout' }), + ).toBeVisible(); + }); + + test('static-nested encoded path with trailing slash', async ({ page }) => { + await page.goto(`http://localhost:${port}/static-nested/encoded%20path/`); + await expect( + page.getByRole('heading', { name: 'Nested / encoded%20path' }), + ).toBeVisible(); + }); + test('check hydration error', async ({ page }) => { const messages: string[] = []; page.on('console', (msg) => messages.push(msg.text())); @@ -85,6 +104,12 @@ test.describe('fs-router', () => { expect(await res.text()).toBe('Hello from API!'); }); + test('api hi with trailing slash', async () => { + const res = await fetch(`http://localhost:${port}/hi/`); + expect(res.status).toBe(200); + expect(await res.text()).toBe('Hello from API!'); + }); + test('api hi.txt', async () => { const res = await fetch(`http://localhost:${port}/hi.txt`); expect(res.status).toBe(200); @@ -106,6 +131,15 @@ test.describe('fs-router', () => { expect(await res.text()).toBe('POST Hello from API! from the test!'); }); + test('api hi with POST and trailing slash', async () => { + const res = await fetch(`http://localhost:${port}/hi/`, { + method: 'POST', + body: 'from the test!', + }); + expect(res.status).toBe(200); + expect(await res.text()).toBe('POST Hello from API! from the test!'); + }); + test('api has-default GET', async () => { const res = await fetch(`http://localhost:${port}/has-default`); expect(res.status).toBe(200); diff --git a/e2e/router-client-no-404.spec.ts b/e2e/router-client-no-404.spec.ts index 7e4faaf603..a8d5e99ac8 100644 --- a/e2e/router-client-no-404.spec.ts +++ b/e2e/router-client-no-404.spec.ts @@ -33,4 +33,21 @@ test.describe('router-client-no-404', () => { ); await expect(page).toHaveURL(/\/missing$/); }); + + test('client navigation to missing trailing-slash route renders Not Found fallback without /404 page', async ({ + page, + }) => { + await page.goto(`http://localhost:${port}/start`); + await waitForHydration(page); + + await page.getByTestId('go-missing-trailing').click(); + + await expect( + page.getByRole('heading', { name: 'Not Found' }), + ).toBeVisible(); + await expect(page.getByRole('heading', { name: 'Custom 404' })).toHaveCount( + 0, + ); + await expect(page).toHaveURL(/\/missing\/$/); + }); }); diff --git a/e2e/router-client.spec.ts b/e2e/router-client.spec.ts index 65769b0d51..13a14fd18d 100644 --- a/e2e/router-client.spec.ts +++ b/e2e/router-client.spec.ts @@ -193,6 +193,26 @@ test.describe('router-client', () => { expect(scrollToCalls).toHaveLength(0); }); + test('same-route trailing-slash navigation preserves scroll and keeps canonical route path', async ({ + page, + }) => { + await page.goto(`http://localhost:${port}/start`); + await waitForHydration(page); + + await page.evaluate(() => { + window.scrollTo({ left: 0, top: 600 }); + }); + await installScrollToRecorder(page); + + await page.getByTestId('router-push-trailing-slash').click(); + + await expect(page.getByTestId('route-path')).toHaveText('/start'); + await expect(page.getByTestId('route-query')).toHaveText(''); + await expect(page.getByTestId('route-hash')).toHaveText(''); + await expect(page).toHaveURL(/\/start\/$/); + expect(await getScrollToCalls(page)).toHaveLength(0); + }); + test('path-change link navigation resets scroll position to top by default', async ({ page, }) => { diff --git a/e2e/use-router.spec.ts b/e2e/use-router.spec.ts index 6426207bd7..f2b3a37645 100644 --- a/e2e/use-router.spec.ts +++ b/e2e/use-router.spec.ts @@ -29,6 +29,20 @@ test.describe('useRouter', () => { await expect(page.getByRole('heading', { name: 'Static' })).toBeVisible(); await expect(page.getByTestId('path')).toHaveText('Path: /static'); }); + + test(`on dynamic pages with trailing slash`, async ({ page }) => { + await page.goto(`http://localhost:${port}/dynamic/`); + await expect( + page.getByRole('heading', { name: 'Dynamic' }), + ).toBeVisible(); + await expect(page.getByTestId('path')).toHaveText('Path: /dynamic'); + }); + + test(`on static pages with trailing slash`, async ({ page }) => { + await page.goto(`http://localhost:${port}/static/`); + await expect(page.getByRole('heading', { name: 'Static' })).toBeVisible(); + await expect(page.getByTestId('path')).toHaveText('Path: /static'); + }); }); test.describe('updates path on link navigation', () => { @@ -63,7 +77,9 @@ test.describe('useRouter', () => { }) => { await page.goto(`http://localhost:${port}/dynamic`); await waitForHydration(page); - await page.getByRole('link', { name: 'Go to static' }).click(); + await page + .getByRole('link', { name: 'Go to static', exact: true }) + .click(); await expect(page.getByRole('heading', { name: 'Static' })).toBeVisible(); const beforeReplaceHistoryLength = await page.evaluate( () => window.history.length, @@ -97,7 +113,9 @@ test.describe('useRouter', () => { await page.click('text=Increment query'); await expect(page.getByTestId('query')).toHaveText('Query: 1'); - await page.getByRole('link', { name: 'Go to static' }).click(); + await page + .getByRole('link', { name: 'Go to static', exact: true }) + .click(); await expect(page.getByRole('heading', { name: 'Static' })).toBeVisible(); await expect(page.getByTestId('query')).toHaveText('Query: 0'); @@ -174,6 +192,26 @@ test.describe('useRouter', () => { await expect(page.getByRole('heading', { name: 'Bar' })).toBeVisible(); expect(messages.some((msg) => msg.includes('Uncaught'))).toBe(false); }); + + test('router.push preserves trailing-slash URL while keeping canonical path', async ({ + page, + }) => { + await page.goto(`http://localhost:${port}/dynamic`); + await waitForHydration(page); + await page.getByTestId('router-push-static-trailing').click(); + await expect(page.getByRole('heading', { name: 'Static' })).toBeVisible(); + await expect(page).toHaveURL(`http://localhost:${port}/static/`); + await expect(page.getByTestId('path')).toHaveText('Path: /static'); + }); + + test('Link supports trailing-slash targets', async ({ page }) => { + await page.goto(`http://localhost:${port}/dynamic`); + await waitForHydration(page); + await page.getByTestId('link-static-trailing').click(); + await expect(page.getByRole('heading', { name: 'Static' })).toBeVisible(); + await expect(page).toHaveURL(`http://localhost:${port}/static/`); + await expect(page.getByTestId('path')).toHaveText('Path: /static'); + }); }); test.describe('retrieves query variables', () => { diff --git a/examples/22_define-router/src/waku.server.tsx b/examples/22_define-router/src/waku.server.tsx index 59743dbe0f..5402c67ac7 100644 --- a/examples/22_define-router/src/waku.server.tsx +++ b/examples/22_define-router/src/waku.server.tsx @@ -133,7 +133,7 @@ export default adapter( // using `[slug]` syntax is just an example and it technically conflicts with others. So, it's better to use a different prefix like `dynamic-page:`. 'page:/dynamic/[slug]': { isStatic: false, - renderer: ({ pathname }) =>

    {pathname}

    , + renderer: ({ routePath }) =>

    {routePath}

    , }, }, }, diff --git a/packages/waku/src/router/client.tsx b/packages/waku/src/router/client.tsx index cb2e5f5749..4f6360fecc 100644 --- a/packages/waku/src/router/client.tsx +++ b/packages/waku/src/router/client.tsx @@ -41,14 +41,19 @@ import { SKIP_HEADER, encodeRoutePath, encodeSliceId, + pathnameToRoutePath, } from './common.js'; import type { RouteProps } from './common.js'; +type AllowTrailingSlash = Path extends '/' + ? Path + : Path | `${Path}/`; + type AllowPathDecorators = Path extends unknown ? - | Path - | `${Path}?${string}` - | `${Path}#${string}` + | AllowTrailingSlash + | `${AllowTrailingSlash}?${string}` + | `${AllowTrailingSlash}#${string}` | `?${string}` | `#${string}` : never; @@ -59,20 +64,15 @@ type InferredPaths = RouteConfig extends { ? AllowPathDecorators : string; -const normalizeRoutePath = (path: string) => { - path = removeBase(path, import.meta.env.WAKU_CONFIG_BASE_PATH); - for (const suffix of ['/', '/index.html']) { - if (path.endsWith(suffix)) { - return path.slice(0, -suffix.length) || '/'; - } - } - return path; -}; +const pathnameToCurrentRoutePath = (pathname: string) => + pathnameToRoutePath( + removeBase(pathname, import.meta.env.WAKU_CONFIG_BASE_PATH), + ); const parseRoute = (url: URL): RouteProps => { const { pathname, searchParams, hash } = url; return { - path: normalizeRoutePath(pathname), + path: pathnameToCurrentRoutePath(pathname), query: searchParams.toString(), hash, }; @@ -107,7 +107,8 @@ const parseRouteFromLocation = (): RouteProps => { }; const shouldScrollByDefault = (url: URL) => - url.pathname !== window.location.pathname || + pathnameToCurrentRoutePath(url.pathname) !== + pathnameToCurrentRoutePath(window.location.pathname) || url.hash !== window.location.hash; const isPathChange = (next: RouteProps, prev: RouteProps) => @@ -141,7 +142,7 @@ const createRscParams = (query: string): URLSearchParams => { type ChangeRouteOptions = { shouldScroll: boolean; - skipRefetch?: boolean; + refetch?: boolean; // true: force refetch, false: don't refetch, undefined: auto-decide based on route change mode?: undefined | 'push' | 'replace'; url?: URL | undefined; unstable_startTransition?: ((fn: TransitionFunction) => void) | undefined; @@ -260,7 +261,7 @@ export function useRouter() { ); const reload = useCallback(async () => { const url = new URL(window.location.href); - await changeRoute(parseRoute(url), { shouldScroll: true }); + await changeRoute(parseRoute(url), { shouldScroll: true, refetch: true }); }, [changeRoute]); const back = useCallback(() => { // FIXME is this correct? @@ -812,14 +813,12 @@ const InnerRouter = ({ routeChangeListenersRef.current = createRouteChangeListeners(); } // Update the route post-load to include the current hash. + const routeRef = useRef(route); useEffect(() => { + routeRef.current = initialRoute; setRoute((prev) => (isSameRoute(prev, initialRoute) ? prev : initialRoute)); }, [initialRoute]); const [err, setErr] = useState(null); - const routeRef = useRef(route); - useEffect(() => { - routeRef.current = route; - }, [route]); const [routeChangeEvents, emitRouteChangeEvent] = routeChangeListenersRef.current; @@ -833,7 +832,6 @@ const InnerRouter = ({ emitRouteChangeEvent('start', nextRoute); const startTransitionFn = options.unstable_startTransition || ((fn: TransitionFunction) => fn()); - const { skipRefetch } = options; const historyPathnameBeforeChange = window.location.pathname; const writeHistoryIfNeeded = ( mode: undefined | 'push' | 'replace', @@ -854,9 +852,11 @@ const InnerRouter = ({ let { mode, url } = options; const requestedUrlToWrite = mode && (url || getRouteUrl(nextRoute)); const routeBeforeChange = routeRef.current; + const shouldRefetch = + options.refetch ?? !isSameRoute(nextRoute, routeBeforeChange); const pathChanged = isPathChange(nextRoute, routeBeforeChange); setErr(null); - if (!staticPathSetRef.current.has(nextRoute.path) && !skipRefetch) { + if (!staticPathSetRef.current.has(nextRoute.path) && shouldRefetch) { const rscPath = encodeRoutePath(nextRoute.path); const rscParams = createRscParams(nextRoute.query); const skipHeaderEnhancer = @@ -920,6 +920,7 @@ const InnerRouter = ({ return; } writeHistoryIfNeeded(mode, urlToWrite); + routeRef.current = nextRoute; setRoute(nextRoute); routeChangeAbortRef.current = null; if (options.shouldScroll) { @@ -948,7 +949,7 @@ const InnerRouter = ({ url.search = query; url.hash = ''; await changeRoute(parseRoute(url), { - skipRefetch: true, + refetch: false, shouldScroll: false, mode: path === '/404' ? undefined : 'push', url, @@ -1003,7 +1004,6 @@ const InnerRouter = ({ } changeRoute(nextRoute, { shouldScroll: shouldScrollForRouteChange(nextRoute, routeRef.current), - skipRefetch: isSameRoute(nextRoute, routeRef.current), }).catch((err) => { console.log('Error while navigating back:', err); }); diff --git a/packages/waku/src/router/common.ts b/packages/waku/src/router/common.ts index 25b1c5d260..1c4fb9b594 100644 --- a/packages/waku/src/router/common.ts +++ b/packages/waku/src/router/common.ts @@ -7,8 +7,24 @@ export type RouteProps = { hash: string; }; -export function getComponentIds(path: string): readonly string[] { - const pathItems = path.split('/').filter(Boolean); +export function pathnameToRoutePath(pathname: string): string { + if (!pathname.startsWith('/')) { + throw new Error('Pathname must start with `/`: ' + pathname); + } + if (pathname.length > 1 && pathname.endsWith('/')) { + pathname = pathname.slice(0, -1); + } + if (pathname.endsWith('/index.html')) { + pathname = pathname.slice(0, -'/index.html'.length) || '/'; + } + if (pathname.length > 1 && pathname.endsWith('/')) { + pathname = pathname.slice(0, -1); + } + return pathname || '/'; +} + +export function getComponentIds(routePath: string): readonly string[] { + const pathItems = routePath.split('/').filter(Boolean); const idSet = new Set(); for (let index = 0; index <= pathItems.length; ++index) { const id = [...pathItems.slice(0, index), 'layout'].join('/'); @@ -21,20 +37,23 @@ export function getComponentIds(path: string): readonly string[] { const ROUTE_PREFIX = 'R'; const SLICE_PREFIX = 'S/'; -export function encodeRoutePath(path: string): string { - if (!path.startsWith('/')) { - throw new Error('Path must start with `/`: ' + path); +export function encodeRoutePath(routePath: string): string { + if (!routePath.startsWith('/')) { + throw new Error('Route path must start with `/`: ' + routePath); + } + if (routePath.length > 1 && routePath.endsWith('/')) { + throw new Error('Route path must not end with `/`: ' + routePath); } - if (path.length > 1 && path.endsWith('/')) { - throw new Error('Path must not end with `/`: ' + path); + if (routePath.endsWith('/index.html')) { + throw new Error('Route path must not end with `/index.html`: ' + routePath); } - if (path === '/') { + if (routePath === '/') { return ROUTE_PREFIX + '/_root'; } - if (path.startsWith('/_')) { - return ROUTE_PREFIX + '/__' + path.slice(2); + if (routePath.startsWith('/_')) { + return ROUTE_PREFIX + '/__' + routePath.slice(2); } - return ROUTE_PREFIX + path; + return ROUTE_PREFIX + routePath; } export function decodeRoutePath(rscPath: string): string { diff --git a/packages/waku/src/router/create-pages.tsx b/packages/waku/src/router/create-pages.tsx index b26607f3ed..be797807d8 100644 --- a/packages/waku/src/router/create-pages.tsx +++ b/packages/waku/src/router/create-pages.tsx @@ -11,6 +11,7 @@ import { import type { PathSpec } from '../lib/utils/path.js'; import { Children, Slot } from '../minimal/client.js'; import { ErrorBoundary } from '../router/client.js'; +import { pathnameToRoutePath } from './common.js'; import type { AnyPage, GetSlugs, @@ -429,39 +430,41 @@ export const createPages = < if (configured) { throw new Error('createPage no longer available'); } - if (pagePathExists(page.path)) { + const pageRoutePath = pathnameToRoutePath(page.path); + if (pagePathExists(pageRoutePath)) { throw new Error(`Duplicated path: ${page.path}`); } - const pathSpec = parsePathWithSlug(page.path); + const pathSpec = parsePathWithSlug(pageRoutePath); const { numSlugs, numWildcards } = getSlugsAndWildcards(pathSpec); if (page.unstable_disableSSR) { noSsrSet.add(pathSpec); } if (page.exactPath) { - const spec = parseExactPath(page.path); + const routePath = pageRoutePath; + const spec = parseExactPath(routePath); if (page.render === 'static') { - staticPathMap.set(page.path, { + staticPathMap.set(routePath, { literalSpec: spec, }); - const id = joinPath(page.path, 'page').replace(/^\//, ''); + const id = joinPath(routePath, 'page').replace(/^\//, ''); if (page.component) { registerStaticComponent(id, page.component); } } else if (page.component) { - dynamicPagePathMap.set(page.path, [spec, page.component]); + dynamicPagePathMap.set(routePath, [spec, page.component]); } else { - dynamicPagePathMap.set(page.path, [spec, []]); + dynamicPagePathMap.set(routePath, [spec, []]); } } else if (page.render === 'static' && numSlugs === 0) { - const pagePath = getGrouplessPath(page.path); - staticPathMap.set(pagePath, { + const routePath = pathnameToRoutePath(getGrouplessPath(page.path)); + staticPathMap.set(routePath, { literalSpec: pathSpec, }); - const id = joinPath(pagePath, 'page').replace(/^\//, ''); - if (pagePath !== page.path) { - groupPathLookup.set(pagePath, page.path); + const id = joinPath(routePath, 'page').replace(/^\//, ''); + if (routePath !== pageRoutePath) { + groupPathLookup.set(routePath, pageRoutePath); } if (page.component) { registerStaticComponent(id, page.component); @@ -482,36 +485,37 @@ export const createPages = < pathSpec, staticPath, ); - const pagePath = getGrouplessPath(definedPath); - staticPathMap.set(pagePath, { + const routePath = pathnameToRoutePath(getGrouplessPath(definedPath)); + staticPathMap.set(routePath, { literalSpec: pathItems.map((name) => ({ type: 'literal', name })), originalSpec: pathSpec, }); - if (pagePath !== definedPath) { - groupPathLookup.set(pagePath, definedPath); + const definedRoutePath = pathnameToRoutePath(definedPath); + if (routePath !== definedRoutePath) { + groupPathLookup.set(routePath, definedRoutePath); } - const id = joinPath(pagePath, 'page').replace(/^\//, ''); + const id = joinPath(routePath, 'page').replace(/^\//, ''); const WrappedComponent = (props: Record) => createElement(page.component as any, { ...props, ...mapping }); registerStaticComponent(id, WrappedComponent); } } else if (page.render === 'dynamic' && numWildcards === 0) { - const pagePath = getGrouplessPath(page.path); - if (pagePath !== page.path) { - groupPathLookup.set(pagePath, page.path); + const routePath = pathnameToRoutePath(getGrouplessPath(page.path)); + if (routePath !== pageRoutePath) { + groupPathLookup.set(routePath, pageRoutePath); } - dynamicPagePathMap.set(pagePath, [pathSpec, page.component]); + dynamicPagePathMap.set(routePath, [pathSpec, page.component]); } else if (page.render === 'dynamic' && numWildcards === 1) { - const pagePath = getGrouplessPath(page.path); - if (pagePath !== page.path) { - groupPathLookup.set(pagePath, page.path); + const routePath = pathnameToRoutePath(getGrouplessPath(page.path)); + if (routePath !== pageRoutePath) { + groupPathLookup.set(routePath, pageRoutePath); } - wildcardPagePathMap.set(pagePath, [pathSpec, page.component]); + wildcardPagePathMap.set(routePath, [pathSpec, page.component]); } else { throw new Error('Invalid page configuration ' + JSON.stringify(page)); } if (page.slices?.length) { - slicePathMap.set(page.path, page.slices); + slicePathMap.set(pageRoutePath, page.slices); } return page as Exclude; }; @@ -520,15 +524,16 @@ export const createPages = < if (configured) { throw new Error('createLayout no longer available'); } + const routePath = pathnameToRoutePath(layout.path); if (layout.render === 'static') { - const id = joinPath(layout.path, 'layout').replace(/^\//, ''); + const id = joinPath(routePath, 'layout').replace(/^\//, ''); registerStaticComponent(id, layout.component); } else if (layout.render === 'dynamic') { - if (dynamicLayoutPathMap.has(layout.path)) { + if (dynamicLayoutPathMap.has(routePath)) { throw new Error(`Duplicated dynamic path: ${layout.path}`); } - const pathSpec = parsePathWithSlug(layout.path); - dynamicLayoutPathMap.set(layout.path, [pathSpec, layout.component]); + const pathSpec = parsePathWithSlug(routePath); + dynamicLayoutPathMap.set(routePath, [pathSpec, layout.component]); } else { throw new Error('Invalid layout configuration'); } @@ -538,10 +543,11 @@ export const createPages = < if (configured) { throw new Error('createApi no longer available'); } - if (apiPathMap.has(options.path)) { + const routePath = pathnameToRoutePath(options.path); + if (apiPathMap.has(routePath)) { throw new Error(`Duplicated api path: ${options.path}`); } - const pathSpec = parsePathWithSlug(options.path); + const pathSpec = parsePathWithSlug(routePath); if (options.render === 'static') { const { numSlugs, numWildcards } = getSlugsAndWildcards(pathSpec); if (numSlugs > 0 && options.staticPaths) { @@ -556,24 +562,25 @@ export const createPages = < pathSpec, staticPath, ); - if (apiPathMap.has(definedPath)) { + const definedRoutePath = pathnameToRoutePath(definedPath); + if (apiPathMap.has(definedRoutePath)) { throw new Error(`Duplicated api path: ${definedPath}`); } - apiPathMap.set(definedPath, { + apiPathMap.set(definedRoutePath, { render: 'static', pathSpec: pathItems.map((name) => ({ type: 'literal', name })), handlers: { GET: options.handler }, }); } } else { - apiPathMap.set(options.path, { + apiPathMap.set(routePath, { render: 'static', pathSpec, handlers: { GET: options.handler }, }); } } else { - apiPathMap.set(options.path, { + apiPathMap.set(routePath, { render: 'dynamic', pathSpec, handlers: options.handlers, @@ -646,7 +653,7 @@ export const createPages = < type ElementSpec = { isStatic: boolean; renderer: (options: { - pathname: string; + routePath: string; query: string | undefined; }) => ReactNode; }; @@ -691,9 +698,9 @@ export const createPages = < createElement( pageComponent, { - ...getPropsMapping(option.pathname), + ...getPropsMapping(option.routePath), ...(option.query ? { query: option.query } : {}), - path: option.pathname, + path: option.routePath, }, , ), @@ -745,9 +752,9 @@ export const createPages = < isStatic: comp.render === 'static', renderer: (option) => createElement(comp.component, { - ...getPropsMapping(option.pathname), + ...getPropsMapping(option.routePath), ...(option.query ? { query: option.query } : {}), - path: option.pathname, + path: option.routePath, }), }; } @@ -759,9 +766,9 @@ export const createPages = < createElement( components, { - ...getPropsMapping(option.pathname), + ...getPropsMapping(option.routePath), ...(option.query ? { query: option.query } : {}), - path: option.pathname, + path: option.routePath, }, , ), @@ -813,9 +820,9 @@ export const createPages = < isStatic: comp.render === 'static', renderer: (option) => createElement(comp.component, { - ...getPropsMapping(option.pathname), + ...getPropsMapping(option.routePath), ...(option.query ? { query: option.query } : {}), - path: option.pathname, + path: option.routePath, }), }; } @@ -827,9 +834,9 @@ export const createPages = < createElement( components, { - ...getPropsMapping(option.pathname), + ...getPropsMapping(option.routePath), ...(option.query ? { query: option.query } : {}), - path: option.pathname, + path: option.routePath, }, , ), diff --git a/packages/waku/src/router/define-router.tsx b/packages/waku/src/router/define-router.tsx index 080d6cd19a..b9c685baa8 100644 --- a/packages/waku/src/router/define-router.tsx +++ b/packages/waku/src/router/define-router.tsx @@ -16,6 +16,7 @@ import { decodeSliceId, encodeRoutePath, encodeSliceId, + pathnameToRoutePath, } from './common.js'; export type ApiHandler = ( @@ -124,18 +125,19 @@ const is404 = (pathSpec: PathSpec) => pathSpec[0]!.type === 'literal' && pathSpec[0]!.name === '404'; -const pathSpec2pathname = (pathSpec: PathSpec) => { +const pathSpecToRoutePath = (pathSpec: PathSpec) => { if (pathSpec.some(({ type }) => type !== 'literal')) { return undefined; } return '/' + pathSpec.map(({ name }) => name!).join('/'); }; -const htmlPath2pathname = (htmlPath: string): string => - htmlPath === '/404' ? '404.html' : htmlPath + '/index.html'; +const routePathToHtmlFilePath = (routePath: string): string => + routePath === '/404' ? '404.html' : routePath + '/index.html'; export function unstable_rerenderRoute(pathname: string, query?: string) { - const rscPath = encodeRoutePath(pathname); + const routePath = pathnameToRoutePath(pathname); + const rscPath = encodeRoutePath(routePath); getRerender()(rscPath, query && new URLSearchParams({ query })); } @@ -144,7 +146,7 @@ export function unstable_notFound(): never { } export function unstable_redirect( - location: string, + location: string, // only URL `pathname` is supported. status: 303 | 307 | 308 = 307, ): never { throw createCustomError('Redirect', { status, location }); @@ -166,7 +168,7 @@ const assertNonReservedSlotId = (slotId: SlotId) => { } }; -type RendererOption = { pathname: string; query: string | undefined }; +type RendererOption = { routePath: string; query: string | undefined }; type RouteConfig = { type: 'route'; @@ -256,11 +258,12 @@ export function unstable_defineRouter(fns: { }; const getPathConfigItem = async (pathname: string) => { + const routePath = pathnameToRoutePath(pathname); const myConfig = await getMyConfig(); const found = myConfig.configs.find( (item): item is typeof item & { type: 'route' | 'api' } => (item.type === 'route' || item.type === 'api') && - !!getPathMapping(item.path, pathname), + !!getPathMapping(item.path, routePath), ); return found; }; @@ -295,8 +298,8 @@ export function unstable_defineRouter(fns: { ) => { setRscPath(rscPath); setRscParams(rscParams); - const pathname = decodeRoutePath(rscPath); - const pathConfigItem = await getPathConfigItem(pathname); + const routePath = decodeRoutePath(rscPath); + const pathConfigItem = await getPathConfigItem(routePath); if (pathConfigItem?.type !== 'route') { return null; } @@ -308,9 +311,9 @@ export function unstable_defineRouter(fns: { } const skipIdSet = new Set(isStringArray(skipParam) ? skipParam : []); const { query } = parseRscParams(rscParams); - const routeId = ROUTE_SLOT_ID_PREFIX + pathname; + const routeId = ROUTE_SLOT_ID_PREFIX + routePath; const option: RendererOption = { - pathname, + routePath, query: pathConfigItem.isStatic ? undefined : query, }; const myConfig = await getMyConfig(); @@ -386,7 +389,7 @@ export function unstable_defineRouter(fns: { entries[id] = sliceElement; }), ]); - entries[ROUTE_ID] = [pathname, query]; + entries[ROUTE_ID] = [routePath, query]; entries[IS_STATIC_ID] = pathConfigItem.isStatic; sliceConfigMap.forEach((sliceConfig, sliceId) => { if (sliceConfig.isStatic) { @@ -538,7 +541,8 @@ export function unstable_defineRouter(fns: { } catch (e) { const info = getErrorInfo(e); if (info?.location) { - const rscPath = encodeRoutePath(info.location); + const routePath = pathnameToRoutePath(info.location); + const rscPath = encodeRoutePath(routePath); const entries = await getEntriesForRoute( rscPath, undefined, @@ -563,7 +567,8 @@ export function unstable_defineRouter(fns: { query: string, httpstatus = 200, ) => { - const rscPath = encodeRoutePath(pathname); + const routePath = pathnameToRoutePath(pathname); + const rscPath = encodeRoutePath(routePath); const rscParams = new URLSearchParams({ query }); const entries = await getEntriesForRoute( rscPath, @@ -578,7 +583,7 @@ export function unstable_defineRouter(fns: { const path2moduleIds = await getPath2moduleIds(); const html = ( ); @@ -661,15 +666,15 @@ export function unstable_defineRouter(fns: { if (!item.isStatic) { continue; } - const pathname = pathSpec2pathname(item.path); - if (!pathname) { + const routePath = pathSpecToRoutePath(item.path); + if (!routePath) { continue; } - const req = new Request(new URL(pathname, 'http://localhost:3000')); + const req = new Request(new URL(routePath, 'http://localhost:3000')); runTask(async () => { await withRequest(req, async () => { const res = await item.handler(req, { params: {} }); - await generateFile(pathname, res.body || ''); + await generateFile(routePath, res.body || ''); }); }); } @@ -685,12 +690,12 @@ export function unstable_defineRouter(fns: { if (!item.isStatic) { continue; } - const pathname = pathSpec2pathname(item.path); - if (!pathname) { + const routePath = pathSpecToRoutePath(item.path); + if (!routePath) { continue; } - const rscPath = encodeRoutePath(pathname); - const req = new Request(new URL(pathname, 'http://localhost:3000')); + const rscPath = encodeRoutePath(routePath); + const req = new Request(new URL(routePath, 'http://localhost:3000')); runTask(async () => { await withRequest(req, async () => { const entries = await getEntriesForRoute( @@ -719,7 +724,7 @@ export function unstable_defineRouter(fns: { htmlRenderTasks.add(async () => { const html = ( ); @@ -728,7 +733,10 @@ export function unstable_defineRouter(fns: { unstable_extraScriptContent: getRouterPrefetchCode(path2moduleIds), }); - await generateFile(htmlPath2pathname(pathname), res.body || ''); + await generateFile( + routePathToHtmlFilePath(routePath), + res.body || '', + ); }); }); }); @@ -743,12 +751,12 @@ export function unstable_defineRouter(fns: { continue; } if (item.noSsr) { - const pathname = pathSpec2pathname(item.path); - if (!pathname) { + const routePath = pathSpecToRoutePath(item.path); + if (!routePath) { throw new Error('Pathname is required for noSsr routes on build'); } runTask(async () => { - await generateDefaultHtml(htmlPath2pathname(pathname)); + await generateDefaultHtml(routePathToHtmlFilePath(routePath)); }); } } diff --git a/packages/waku/src/router/fs-router.ts b/packages/waku/src/router/fs-router.ts index fc93485503..cf0f3b0a85 100644 --- a/packages/waku/src/router/fs-router.ts +++ b/packages/waku/src/router/fs-router.ts @@ -86,7 +86,7 @@ export function fsRouter( ); } else if (pathItems.at(0) === options.apiDir) { // Strip the apiDir prefix from the path (e.g., _api/hello.txt -> hello.txt) - const apiPath = pathItems.slice(1).join('/'); + const apiPath = '/' + pathItems.slice(1).join('/'); if (config?.render === 'static') { if (Object.keys(mod).length !== 2 || !mod.GET) { console.warn( diff --git a/packages/waku/tests/create-pages.test.ts b/packages/waku/tests/create-pages.test.ts index 392306e491..32b4668c51 100644 --- a/packages/waku/tests/create-pages.test.ts +++ b/packages/waku/tests/create-pages.test.ts @@ -1473,6 +1473,23 @@ describe('createPages pages and layouts', () => { await expect(getConfigs).rejects.toThrowError('Duplicated path: /test'); }); + it('fails if canonical and trailing-slash paths override each other', async () => { + createPages(async ({ createPage }) => [ + createPage({ + render: 'dynamic', + path: '/test', + component: () => null, + }), + createPage({ + render: 'dynamic', + path: '/test/' as never, + component: () => null, + }), + ]); + const { getConfigs } = injectedFunctions(); + await expect(getConfigs).rejects.toThrowError('Duplicated path: /test/'); + }); + it('creates a complex router', async () => { const TestPage = vi.fn(); complexTestRouter(createPages, TestPage); diff --git a/packages/waku/tests/inferred-paths.test.ts b/packages/waku/tests/inferred-paths.test.ts index 642152d628..fe0e898ecc 100644 --- a/packages/waku/tests/inferred-paths.test.ts +++ b/packages/waku/tests/inferred-paths.test.ts @@ -3,11 +3,15 @@ import type { TypeEqual } from 'ts-expect'; import { describe, it } from 'vitest'; // Reproduce the AllowPathDecorators type from router/client.tsx to test it directly +type AllowTrailingSlash = Path extends '/' + ? Path + : Path | `${Path}/`; + type AllowPathDecorators = Path extends unknown ? - | Path - | `${Path}?${string}` - | `${Path}#${string}` + | AllowTrailingSlash + | `${AllowTrailingSlash}?${string}` + | `${AllowTrailingSlash}#${string}` | `?${string}` | `#${string}` : never; @@ -17,6 +21,7 @@ describe('AllowPathDecorators', () => { type Result = AllowPathDecorators<'/' | '/foo'>; expectType>(true); expectType>(true); + expectType>(true); }); it('allows paths with query strings', () => { @@ -27,6 +32,9 @@ describe('AllowPathDecorators', () => { expectType>( true, ); + expectType>( + true, + ); }); it('allows paths with hash fragments', () => { @@ -37,6 +45,9 @@ describe('AllowPathDecorators', () => { expectType>( true, ); + expectType>( + true, + ); }); it('allows bare query strings without a path prefix', () => { diff --git a/packages/waku/tests/path.test.ts b/packages/waku/tests/path.test.ts index 1779739dd8..29e244ae4c 100644 --- a/packages/waku/tests/path.test.ts +++ b/packages/waku/tests/path.test.ts @@ -52,12 +52,14 @@ describe('getPathMapping', () => { test('handles literal paths', () => { const pathSpec = parsePathWithSlug('/foo/bar'); expect(getPathMapping(pathSpec, '/foo/bar')).toEqual({}); + expect(getPathMapping(pathSpec, '/foo/bar/')).toEqual({}); expect(getPathMapping(pathSpec, '/foo/baz')).toBe(null); }); test('handles paths with groups', () => { const pathSpec = parsePathWithSlug('/foo/[id]'); expect(getPathMapping(pathSpec, '/foo/123')).toEqual({ id: '123' }); + expect(getPathMapping(pathSpec, '/foo/123/')).toEqual({ id: '123' }); expect(getPathMapping(pathSpec, '/foo/bar')).toEqual({ id: 'bar' }); expect(getPathMapping(pathSpec, '/foo')).toBe(null); }); diff --git a/packages/waku/tests/router-client.test.tsx b/packages/waku/tests/router-client.test.tsx index 06e110d7d5..748be839db 100644 --- a/packages/waku/tests/router-client.test.tsx +++ b/packages/waku/tests/router-client.test.tsx @@ -260,7 +260,7 @@ afterEach(() => { }); describe('router/client utilities', () => { - test('parses route path/query/hash and normalizes trailing suffixes', () => { + test('parses route path/query/hash and canonicalizes path from pathname', () => { const route = unstable_parseRoute( new URL('http://localhost/foo/index.html?count=2#hash'), ); @@ -276,6 +276,13 @@ describe('router/client utilities', () => { query: 'q=1', hash: '', }); + + const route3 = unstable_parseRoute(new URL('http://localhost/baz/?q=1')); + expect(route3).toEqual({ + path: '/baz', + query: 'q=1', + hash: '', + }); }); test('reads httpstatus meta content', () => { @@ -405,7 +412,7 @@ describe('useRouter + Link with context', () => { expect(changeRoute).toHaveBeenNthCalledWith( 3, { path: '/start', query: '', hash: '' }, - { shouldScroll: true }, + { shouldScroll: true, refetch: true }, ); const firstUrl = ( (changeRoute.mock.calls[0] as unknown[] | undefined)?.[1] as diff --git a/packages/waku/tests/rscPath.test.ts b/packages/waku/tests/rscPath.test.ts index 555d17cea3..01b251979f 100644 --- a/packages/waku/tests/rscPath.test.ts +++ b/packages/waku/tests/rscPath.test.ts @@ -5,7 +5,11 @@ import { encodeFuncId, encodeRscPath, } from '../src/lib/utils/rsc-path.js'; -import { decodeRoutePath, encodeRoutePath } from '../src/router/common.js'; +import { + decodeRoutePath, + encodeRoutePath, + pathnameToRoutePath, +} from '../src/router/common.js'; describe('encodeRscPath', () => { test('encodes rscPath', () => { @@ -79,6 +83,20 @@ describe('decodeFuncId', () => { }); }); +describe('pathnameToRoutePath', () => { + test('canonicalizes trailing slash and index.html', () => { + expect(pathnameToRoutePath('/')).toBe('/'); + expect(pathnameToRoutePath('/foo')).toBe('/foo'); + expect(pathnameToRoutePath('/foo/')).toBe('/foo'); + expect(pathnameToRoutePath('/index.html')).toBe('/'); + expect(pathnameToRoutePath('/foo/index.html')).toBe('/foo'); + }); + + test('throws on invalid pathname', () => { + expect(() => pathnameToRoutePath('foo')).toThrow(); + }); +}); + describe('encodeRoutePath', () => { test('encodes routePath', () => { expect(encodeRoutePath('/')).toBe('R/_root'); @@ -89,6 +107,8 @@ describe('encodeRoutePath', () => { test('throws on invalid routePath', () => { expect(() => encodeRoutePath('foo')).toThrow(); expect(() => encodeRoutePath('/foo/')).toThrow(); + expect(() => encodeRoutePath('/index.html')).toThrow(); + expect(() => encodeRoutePath('/foo/index.html')).toThrow(); }); });