diff --git a/.changeset/blue-buckets-attack.md b/.changeset/blue-buckets-attack.md new file mode 100644 index 000000000..f22c107e0 --- /dev/null +++ b/.changeset/blue-buckets-attack.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Adds a check during build to make sure routing priority is always enforced in the final dist output diff --git a/packages/astro/src/core/build/page-data.ts b/packages/astro/src/core/build/page-data.ts index 18823c462..fee13ea49 100644 --- a/packages/astro/src/core/build/page-data.ts +++ b/packages/astro/src/core/build/page-data.ts @@ -10,7 +10,9 @@ import { debug } from '../logger/core.js'; import { preload as ssrPreload } from '../render/dev/index.js'; import { generateRssFunction } from '../render/rss.js'; import { callGetStaticPaths, RouteCache, RouteCacheEntry } from '../render/route-cache.js'; +import { removeTrailingForwardSlash } from '../path.js'; import { isBuildingToSSR } from '../util.js'; +import { matchRoute } from '../routing/match.js'; export interface CollectPagesDataOptions { astroConfig: AstroConfig; @@ -35,6 +37,7 @@ export async function collectPagesData( const assets: Record = {}; const allPages: AllPagesData = {}; + const builtPaths = new Set(); const buildMode = isBuildingToSSR(astroConfig) ? 'ssr' : 'static'; @@ -60,6 +63,7 @@ export async function collectPagesData( ); clearInterval(routeCollectionLogTimeout); }, 10000); + builtPaths.add(route.pathname); allPages[route.component] = { component: route.component, route, @@ -138,7 +142,28 @@ export async function collectPagesData( } const finalPaths = result.staticPaths .map((staticPath) => staticPath.params && route.generate(staticPath.params)) - .filter(Boolean); + .filter((staticPath) => { + // Remove empty or undefined paths + if (!staticPath) { + return false; + } + + // The path hasn't been built yet, include it + if (!builtPaths.has(removeTrailingForwardSlash(staticPath))) { + return true; + } + + // The path was already built once. Check the manifest to see if + // this route takes priority for the final URL. + // NOTE: The same URL may match multiple routes in the manifest. + // Routing priority needs to be verified here for any duplicate + // paths to ensure routing priority rules are enforced in the final build. + const matchedRoute = matchRoute(staticPath, manifest); + return matchedRoute === route; + }); + + finalPaths.map((staticPath) => builtPaths.add(removeTrailingForwardSlash(staticPath))); + allPages[route.component] = { component: route.component, route, diff --git a/packages/astro/src/vite-plugin-astro-server/index.ts b/packages/astro/src/vite-plugin-astro-server/index.ts index 6ee81593f..f71e28ba0 100644 --- a/packages/astro/src/vite-plugin-astro-server/index.ts +++ b/packages/astro/src/vite-plugin-astro-server/index.ts @@ -199,13 +199,10 @@ async function handleRequest( const devRoot = site ? site.pathname : '/'; const origin = `${viteServer.config.server.https ? 'https' : 'http'}://${req.headers.host}`; const buildingToSSR = isBuildingToSSR(config); - // When file-based build format is used, pages will be built to `/blog.html` - // rather than `/blog/index.html`. The dev server should handle this as well - // to match production deployments. - const url = - config.build.format === 'file' - ? new URL(origin + req.url?.replace(/(index)?\.html$/, '')) - : new URL(origin + req.url); + // Ignore `.html` extensions and `index.html` in request URLS to ensure that + // routing behavior matches production builds. This supports both file and directory + // build formats, and is necessary based on how the manifest tracks build targets. + const url = new URL(origin + req.url?.replace(/(index)?\.html$/, '')); const pathname = decodeURI(url.pathname); const rootRelativeUrl = pathname.substring(devRoot.length - 1); if (!buildingToSSR) { diff --git a/packages/astro/test/fixtures/routing-priority/astro.config.mjs b/packages/astro/test/fixtures/routing-priority/astro.config.mjs new file mode 100644 index 000000000..882e6515a --- /dev/null +++ b/packages/astro/test/fixtures/routing-priority/astro.config.mjs @@ -0,0 +1,4 @@ +import { defineConfig } from 'astro/config'; + +// https://astro.build/config +export default defineConfig({}); diff --git a/packages/astro/test/fixtures/routing-priority/package.json b/packages/astro/test/fixtures/routing-priority/package.json new file mode 100644 index 000000000..01c23a914 --- /dev/null +++ b/packages/astro/test/fixtures/routing-priority/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/routing-priority", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/routing-priority/src/pages/[lang]/[...catchall].astro b/packages/astro/test/fixtures/routing-priority/src/pages/[lang]/[...catchall].astro new file mode 100644 index 000000000..622638e70 --- /dev/null +++ b/packages/astro/test/fixtures/routing-priority/src/pages/[lang]/[...catchall].astro @@ -0,0 +1,23 @@ +--- + export async function getStaticPaths() { + return [ + { params: { lang: 'de', catchall: '1/2' } }, + { params: { lang: 'en', catchall: '1/2' } } + ] + } +--- + + + + + + + Routing + + + +

[lang]/[...catchall].astro

+

{Astro.params.lang} | {Astro.params.catchall}

+ + + diff --git a/packages/astro/test/fixtures/routing-priority/src/pages/[lang]/index.astro b/packages/astro/test/fixtures/routing-priority/src/pages/[lang]/index.astro new file mode 100644 index 000000000..a0bf17881 --- /dev/null +++ b/packages/astro/test/fixtures/routing-priority/src/pages/[lang]/index.astro @@ -0,0 +1,23 @@ +--- + export async function getStaticPaths() { + return [ + { params: { lang: 'de' } }, // always shadowed by /de/index.astro + { params: { lang: 'en' } } + ]; + } +--- + + + + + + + Routing + + + +

[lang]/index.astro

+

{Astro.params.lang}

+ + + diff --git a/packages/astro/test/fixtures/routing-priority/src/pages/de/index.astro b/packages/astro/test/fixtures/routing-priority/src/pages/de/index.astro new file mode 100644 index 000000000..163ce8995 --- /dev/null +++ b/packages/astro/test/fixtures/routing-priority/src/pages/de/index.astro @@ -0,0 +1,12 @@ +--- +--- + + + + + Routing + + +

de/index.astro

+ + diff --git a/packages/astro/test/fixtures/routing-priority/src/pages/index.astro b/packages/astro/test/fixtures/routing-priority/src/pages/index.astro new file mode 100644 index 000000000..b89f4ab03 --- /dev/null +++ b/packages/astro/test/fixtures/routing-priority/src/pages/index.astro @@ -0,0 +1,12 @@ +--- +--- + + + + + Routing + + +

index.astro

+ + diff --git a/packages/astro/test/fixtures/routing-priority/src/pages/posts/[...slug].astro b/packages/astro/test/fixtures/routing-priority/src/pages/posts/[...slug].astro new file mode 100644 index 000000000..839a46f9b --- /dev/null +++ b/packages/astro/test/fixtures/routing-priority/src/pages/posts/[...slug].astro @@ -0,0 +1,24 @@ +--- + export async function getStaticPaths() { + return [ + { + params: { slug: "1/2" }, + } + ] + } +--- + + + + + + + Routing + + + +

posts/[...slug].astro

+

{Astro.params.slug}

+ + + diff --git a/packages/astro/test/fixtures/routing-priority/src/pages/posts/[pid].astro b/packages/astro/test/fixtures/routing-priority/src/pages/posts/[pid].astro new file mode 100644 index 000000000..47b332a32 --- /dev/null +++ b/packages/astro/test/fixtures/routing-priority/src/pages/posts/[pid].astro @@ -0,0 +1,23 @@ +--- + export async function getStaticPaths() { + return [ + { params: { pid: 'post-1' } }, + { params: { pid: 'post-2' } } + ] + } +--- + + + + + + + Routing + + + +

posts/[pid].astro

+

{Astro.params.pid}

+ + + diff --git a/packages/astro/test/routing-priority.test.js b/packages/astro/test/routing-priority.test.js new file mode 100644 index 000000000..1075ea5b1 --- /dev/null +++ b/packages/astro/test/routing-priority.test.js @@ -0,0 +1,236 @@ +import { expect } from 'chai'; +import { load as cheerioLoad } from 'cheerio'; +import path from 'path'; +import { loadFixture } from './test-utils.js'; + +let fixture; + +const routes = [ + { + url: '/', + h1: 'index.astro' + }, + { + url: '/posts/post-1', + h1: 'posts/[pid].astro', + p: 'post-1' + }, + { + url: '/posts/post-2', + h1: 'posts/[pid].astro', + p: 'post-2' + }, + { + url: '/posts/1/2', + h1: 'posts/[...slug].astro', + p: '1/2' + }, + { + url: '/de', + h1: 'de/index.astro' + }, + { + url: '/de/', + h1: 'de/index.astro' + }, + { + url: '/de/index.html', + h1: 'de/index.astro' + }, + { + url: '/en', + h1: '[lang]/index.astro', + p: 'en' + }, + { + url: '/en/', + h1: '[lang]/index.astro', + p: 'en' + }, + { + url: '/en/index.html', + h1: '[lang]/index.astro', + p: 'en' + }, + { + url: '/de/1/2', + h1: '[lang]/[...catchall].astro', + p: 'de | 1/2' + }, + { + url: '/en/1/2', + h1: '[lang]/[...catchall].astro', + p: 'en | 1/2' + } +] + +describe('Routing priority', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/routing-priority/', + }); + }); + + describe('build', () => { + before(async () => { + await fixture.build(); + }); + + it('matches / to index.astro', async () => { + const html = await fixture.readFile('/index.html'); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('index.astro'); + }); + + it('matches /posts/post-1 to posts/[pid].astro', async () => { + const html = await fixture.readFile('/posts/post-1/index.html'); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('posts/[pid].astro'); + expect($('p').text()).to.equal('post-1'); + }); + + it('matches /posts/1/2 to posts/[...slug].astro', async () => { + const html = await fixture.readFile('/posts/1/2/index.html'); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('posts/[...slug].astro'); + expect($('p').text()).to.equal('1/2'); + }); + + it('matches /de to de/index.astro', async () => { + const html = await fixture.readFile('/de/index.html'); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('de/index.astro'); + }); + + it('matches /en to [lang]/index.astro', async () => { + const html = await fixture.readFile('/en/index.html'); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('[lang]/index.astro'); + expect($('p').text()).to.equal('en'); + }); + + it('matches /de/1/2 to [lang]/[...catchall].astro', async () => { + const html = await fixture.readFile('/de/1/2/index.html'); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('[lang]/[...catchall].astro'); + expect($('p').text()).to.equal('de | 1/2') + }); + + it('matches /en/1/2 to [lang]/[...catchall].astro', async () => { + const html = await fixture.readFile('/en/1/2/index.html'); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('[lang]/[...catchall].astro'); + expect($('p').text()).to.equal('en | 1/2') + }); + }); + + describe('dev', () => { + let devServer; + + before(async () => { + devServer = await fixture.startDevServer(); + }); + + after(async () => { + await devServer.stop(); + }); + + it('matches / to index.astro', async () => { + const html = await fixture.fetch('/').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('index.astro'); + }); + + it('matches /posts/post-1 to /posts/[pid].astro', async () => { + const html = await fixture.fetch('/posts/post-1').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('posts/[pid].astro'); + expect($('p').text()).to.equal('post-1'); + }); + + it('matches /posts/1/2 to /posts/[...slug].astro', async () => { + const html = await fixture.fetch('/posts/1/2').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('posts/[...slug].astro'); + expect($('p').text()).to.equal('1/2'); + }); + + it('matches /de to de/index.astro', async () => { + const html = await fixture.fetch('/de').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('de/index.astro'); + }); + + it('matches /de to de/index.astro', async () => { + const html = await fixture.fetch('/de').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('de/index.astro'); + }); + + it('matches /de/ to de/index.astro', async () => { + const html = await fixture.fetch('/de/').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('de/index.astro'); + }); + + it('matches /de/index.html to de/index.astro', async () => { + const html = await fixture.fetch('/de/index.html').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('de/index.astro'); + }); + + it('matches /en to [lang]/index.astro', async () => { + const html = await fixture.fetch('/en').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('[lang]/index.astro'); + expect($('p').text()).to.equal('en'); + }); + + it('matches /en/ to [lang]/index.astro', async () => { + const html = await fixture.fetch('/en/').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('[lang]/index.astro'); + expect($('p').text()).to.equal('en'); + }); + + it('matches /en/index.html to de/index.astro', async () => { + const html = await fixture.fetch('/en/index.html').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('[lang]/index.astro'); + expect($('p').text()).to.equal('en'); + }); + + it('matches /de/1/2 to [lang]/[...catchall].astro', async () => { + const html = await fixture.fetch('/de/1/2/index.html').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('[lang]/[...catchall].astro'); + expect($('p').text()).to.equal('de | 1/2'); + }); + + it('matches /en/1/2 to [lang]/[...catchall].astro', async () => { + const html = await fixture.fetch('/en/1/2/index.html').then((res) => res.text()); + const $ = cheerioLoad(html); + + expect($('h1').text()).to.equal('[lang]/[...catchall].astro'); + expect($('p').text()).to.equal('en | 1/2'); + }); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 72af65707..9117c40c7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1162,6 +1162,12 @@ importers: dependencies: astro: link:../../.. + packages/astro/test/fixtures/routing-priority: + specifiers: + astro: workspace:* + dependencies: + astro: link:../../.. + packages/astro/test/fixtures/sass: specifiers: astro: workspace:*