archive/rsnext
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Remove auto appending xml extension to dynamic sitemap routes (#65507)

Browse source 
### What

Remove the auto appending `.xml` extension to the sitemap routes when
it's a dynamic route.


### Why

Previously we were adding `.xml` to `/[...paths/]sitemap` routes, but
the bad part is when you use it to generate multiple sitemaps with
`generateSitemaps` in format like `/[...paths/]sitemap.xml/[id]`, which
doesn't look good in url format and it can be inferred as xml with
content-type. Hence we don't need to add `.xml` in the url.

Before this change it could also result into the different url between
dev and prod:
dev: `/sitemap.xml/[id]`
prod: `/sitemap/[id].xml` 

Now it's going to be aligned as `/sitemap/[id]`. Users can add extension
flexiblely.

Closes NEXT-3357
This commit is contained in:
Jiachi Liu 2024-05-09 11:05:24 +02:00 committed by GitHub
parent 1277ff4d43
commit 09baadfd70
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 115 additions and 117 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
docs/02-app/02-api-reference/02-file-conventions/01-metadata/sitemap.mdx
View file

@ -262,9 +262,7 @@ export default async function sitemap({ id }) {
}
```
In production, your generated sitemaps will be available at `/.../sitemap/[id].xml`. For example, `/product/sitemap/1.xml`.
In development, you can view the generated sitemap on `/.../sitemap.xml/[id]`. For example, `/product/sitemap.xml/1`. This difference is temporary and will follow the production format.
Your generated sitemaps will be available at `/.../sitemap/[id]`. For example, `/product/sitemap/1`.
See the [`generateSitemaps` API reference](/docs/app/api-reference/functions/generate-sitemaps) for more information.

5
packages/next-swc/crates/next-core/src/next_app/metadata/mod.rs
View file

@ -306,9 +306,8 @@ pub fn normalize_metadata_route(mut page: AppPage) -> Result<AppPage> {
route += ".txt"
} else if route == "/manifest" {
route += ".webmanifest"
} else if route.ends_with("/sitemap") {
route += ".xml"
} else {
// Do not append the suffix for the sitemap route
} else if !route.ends_with("/sitemap") {
// Remove the file extension, e.g. /route-path/robots.txt -> /route-path
let pathname_prefix = split_directory(&route).0.unwrap_or_default();
suffix = get_metadata_route_suffix(pathname_prefix);

2
packages/next-swc/crates/next-core/src/next_app/metadata/route.rs
View file

@ -227,7 +227,7 @@ async fn dynamic_site_map_route_source(
const params = []
for (const item of sitemaps) {
params.push({ __metadata_id__: item.id.toString() + '.xml' })
params.push({ __metadata_id__: item.id.toString() })
}
return params
}

4
packages/next/src/build/index.ts
View file

@ -998,13 +998,13 @@ export default async function build(
}
if (
pageKey.includes('sitemap.xml/[[...__metadata_id__]]') &&
pageKey.includes('sitemap/[[...__metadata_id__]]') &&
isDynamic
) {
delete mappedAppPages[pageKey]
mappedAppPages[
pageKey.replace(
'sitemap.xml/[[...__metadata_id__]]',
'sitemap/[[...__metadata_id__]]',
'sitemap/[__metadata_id__]'
)
] = pagePath

5
packages/next/src/build/webpack/loaders/next-metadata-route-loader.ts
View file

@ -176,7 +176,7 @@ export async function generateStaticParams() {
const params = []
for (const item of sitemaps) {
params.push({ __metadata_id__: item.id.toString() + '.xml' })
params.push({ __metadata_id__: item.id.toString() })
}
return params
}
@ -225,9 +225,6 @@ export async function GET(_, ctx) {
}
}
let itemID = item.id.toString()
if(process.env.NODE_ENV === 'production') {
itemID += '.xml'
}
return itemID === targetId
})?.id
if (id == null) {

6
packages/next/src/lib/metadata/get-metadata-route.ts
View file

@ -65,9 +65,9 @@ export function normalizeMetadataRoute(page: string) {
route += '.txt'
} else if (page === '/manifest') {
route += '.webmanifest'
} else if (page.endsWith('/sitemap')) {
route += '.xml'
} else {
}
// For sitemap, we don't automatically add the route suffix since it can have sub-routes
else if (!page.endsWith('/sitemap')) {
// Remove the file extension, e.g. /route-path/robots.txt -> /route-path
const pathnamePrefix = page.slice(0, -(path.basename(page).length + 1))
suffix = getMetadataRouteSuffix(pathnamePrefix)

35
test/e2e/app-dir/dynamic-in-generate-params/index.test.ts
View file

@ -1,39 +1,40 @@
import { type NextInstance, nextTestSetup } from 'e2e-utils'
import type { Response } from 'node-fetch'
async function getLastModifiedTime(next: NextInstance, pathname: string) {
const content = await (await next.fetch(pathname)).text()
return content.match(/<lastmod>([^<]+)<\/lastmod>/)[1]
}
function assertSitemapResponse(res: Response) {
expect(res.status).toBe(200)
expect(res.headers.get('content-type')).toContain('application/xml')
}
describe('app-dir - dynamic in generate params', () => {
const { next, isNextDev } = nextTestSetup({
const { next } = nextTestSetup({
files: __dirname,
skipDeployment: true,
})
it('should render sitemap with generateSitemaps in force-dynamic config dynamically', async () => {
const firstTime = await getLastModifiedTime(
next,
isNextDev ? 'sitemap.xml/0' : '/sitemap/0.xml'
)
const secondTime = await getLastModifiedTime(
next,
isNextDev ? 'sitemap.xml/0' : '/sitemap/0.xml'
)
const firstTime = await getLastModifiedTime(next, 'sitemap/0')
const secondTime = await getLastModifiedTime(next, 'sitemap/0')
expect(firstTime).not.toEqual(secondTime)
})
it('should be able to call while generating multiple dynamic sitemaps', async () => {
expect(
(await next.fetch(isNextDev ? 'sitemap.xml/0' : '/sitemap/0.xml')).status
).toBe(200)
expect(
(await next.fetch(isNextDev ? 'sitemap.xml/1' : '/sitemap/1.xml')).status
).toBe(200)
const res0 = await next.fetch('sitemap/0')
const res1 = await next.fetch('sitemap/1')
assertSitemapResponse(res0)
assertSitemapResponse(res1)
})
it('should be able to call fetch while generating multiple dynamic pages', async () => {
expect((await next.fetch('/dynamic/0')).status).toBe(200)
expect((await next.fetch('/dynamic/1')).status).toBe(200)
const pageRes0 = await next.fetch('dynamic/0')
const pageRes1 = await next.fetch('dynamic/1')
expect(pageRes0.status).toBe(200)
expect(pageRes1.status).toBe(200)
})
})

133
test/e2e/app-dir/metadata-dynamic-routes/index.test.ts
View file

@ -18,7 +18,7 @@ describe('app dir - metadata dynamic routes', () => {
},
})
describe('text routes', () => {
describe('robots.txt', () => {
it('should handle robots.[ext] dynamic routes', async () => {
const res = await next.fetch('/robots.txt')
const text = await res.text()
@ -40,39 +40,53 @@ describe('app dir - metadata dynamic routes', () => {
"
`)
})
})
describe('sitemap', () => {
it('should handle sitemap.[ext] dynamic routes', async () => {
const res = await next.fetch('/sitemap.xml')
const res = await next.fetch('/sitemap')
const text = await res.text()
expect(res.headers.get('content-type')).toBe('application/xml')
expect(res.headers.get('cache-control')).toBe(CACHE_HEADERS.REVALIDATE)
expect(text).toMatchInlineSnapshot(`
"<?xml version="1.0" encoding="UTF-8"?>
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
<url>
<loc>https://example.com</loc>
<lastmod>2021-01-01</lastmod>
<changefreq>weekly</changefreq>
<priority>0.5</priority>
</url>
<url>
<loc>https://example.com/about</loc>
<lastmod>2021-01-01</lastmod>
</url>
</urlset>
"
`)
"<?xml version="1.0" encoding="UTF-8"?>
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
<url>
<loc>https://example.com</loc>
<lastmod>2021-01-01</lastmod>
<changefreq>weekly</changefreq>
<priority>0.5</priority>
</url>
<url>
<loc>https://example.com/about</loc>
<lastmod>2021-01-01</lastmod>
</url>
</urlset>
"
`)
})
it('should not throw if client components are imported but not used', async () => {
const { status } = await next.fetch('/client-ref-dependency/sitemap.xml')
it('should support generate multi sitemaps with generateSitemaps', async () => {
const ids = ['child0', 'child1', 'child2', 'child3']
function fetchSitemap(id) {
return next.fetch(`/gsp/sitemap/${id}`).then((res) => res.text())
}
for (const id of ids) {
const text = await fetchSitemap(id)
expect(text).toContain(`<loc>https://example.com/dynamic/${id}</loc>`)
}
})
it('should not throw if client components are imported but not used in sitemap', async () => {
const { status } = await next.fetch('/client-ref-dependency/sitemap')
expect(status).toBe(200)
})
it('should support alternate.languages in sitemap', async () => {
const xml = await (await next.fetch('/lang/sitemap.xml')).text()
const xml = await (await next.fetch('/lang/sitemap')).text()
expect(xml).toContain('xmlns:xhtml="http://www.w3.org/1999/xhtml')
expect(xml).toContain(
@ -82,6 +96,35 @@ describe('app dir - metadata dynamic routes', () => {
`<xhtml:link rel="alternate" hreflang="de" href="https://example.com/de/about" />`
)
})
if (isNextStart) {
it('should optimize routes without multiple generation API as static routes', async () => {
const appPathsManifest = JSON.parse(
await next.readFile('.next/server/app-paths-manifest.json')
)
expect(appPathsManifest).toMatchObject({
// static routes
'/twitter-image/route': 'app/twitter-image/route.js',
'/sitemap/route': 'app/sitemap/route.js',
// dynamic
'/gsp/sitemap/[__metadata_id__]/route':
'app/gsp/sitemap/[__metadata_id__]/route.js',
'/(group)/dynamic/[size]/apple-icon-ahg52g/[[...__metadata_id__]]/route':
'app/(group)/dynamic/[size]/apple-icon-ahg52g/[[...__metadata_id__]]/route.js',
})
})
it('should generate static paths of dynamic sitemap in production', async () => {
const sitemapPaths = ['child0', 'child1', 'child2', 'child3'].map(
(id) => `.next/server/app/gsp/sitemap/${id}.meta`
)
const promises = sitemapPaths.map(async (filePath) => {
expect(await next.hasFile(filePath)).toBe(true)
})
await Promise.all(promises)
})
}
})
describe('social image routes', () => {
@ -203,22 +246,6 @@ describe('app dir - metadata dynamic routes', () => {
])
})
it('should support generate multi sitemaps with generateSitemaps', async () => {
const ids = ['child0', 'child1', 'child2', 'child3']
function fetchSitemap(id) {
return next
.fetch(
isNextDev ? `/gsp/sitemap.xml/${id}` : `/gsp/sitemap/${id}.xml`
)
.then((res) => res.text())
}
for (const id of ids) {
const text = await fetchSitemap(id)
expect(text).toContain(`<loc>https://example.com/dynamic/${id}</loc>`)
}
})
it('should fill params into dynamic routes url of metadata images', async () => {
const $ = await next.render$('/dynamic/big')
const ogImageUrl = $('meta[property="og:image"]').attr('content')
@ -289,7 +316,7 @@ describe('app dir - metadata dynamic routes', () => {
if (isNextStart) {
describe('route segment config', () => {
it('should generate dynamic route if dynamic config is force-dynamic', async () => {
const dynamicRoute = '/route-config/sitemap.xml'
const dynamicRoute = '/route-config/sitemap'
expect(
await next.hasFile(`.next/server/app${dynamicRoute}/route.js`)
@ -480,7 +507,7 @@ describe('app dir - metadata dynamic routes', () => {
const outputBeforeFetch = next.cliOutput + ''
await next.patchFile(sitemapFilePath, contentMissingIdProperty)
await next.fetch('/metadata-base/unset/sitemap.xml/0')
await next.fetch('/metadata-base/unset/sitemap/0')
const outputAfterFetch = next.cliOutput + ''
const output = outputAfterFetch.replace(outputBeforeFetch, '')
@ -494,7 +521,7 @@ describe('app dir - metadata dynamic routes', () => {
}, /success/)
} finally {
await next.deleteFile(sitemapFilePath)
await next.fetch('/metadata-base/unset/sitemap.xml/0')
await next.fetch('/metadata-base/unset/sitemap/0')
}
})
@ -537,34 +564,6 @@ describe('app dir - metadata dynamic routes', () => {
expect(edgeRoute).toMatch(/\/\(group\)\/twitter-image-\w{6}\/route/)
})
it('should optimize routes without multiple generation API as static routes', async () => {
const appPathsManifest = JSON.parse(
await next.readFile('.next/server/app-paths-manifest.json')
)
expect(appPathsManifest).toMatchObject({
// static routes
'/twitter-image/route': 'app/twitter-image/route.js',
'/sitemap.xml/route': 'app/sitemap.xml/route.js',
// dynamic
'/gsp/sitemap/[__metadata_id__]/route':
'app/gsp/sitemap/[__metadata_id__]/route.js',
'/(group)/dynamic/[size]/apple-icon-ahg52g/[[...__metadata_id__]]/route':
'app/(group)/dynamic/[size]/apple-icon-ahg52g/[[...__metadata_id__]]/route.js',
})
})
it('should generate static paths of dynamic sitemap in production', async () => {
const sitemapPaths = ['child0', 'child1', 'child2', 'child3'].map(
(id) => `.next/server/app/gsp/sitemap/${id}.xml.meta`
)
const promises = sitemapPaths.map(async (filePath) => {
expect(await next.hasFile(filePath)).toBe(true)
})
await Promise.all(promises)
})
it('should include default og font files in file trace', async () => {
const fileTrace = JSON.parse(
await next.readFile(

26
test/turbopack-build-tests-manifest.json
View file

@ -1871,14 +1871,12 @@
"failed": [
"app dir - metadata dynamic routes icon image routes should render apple icon with dynamic routes",
"app dir - metadata dynamic routes icon image routes should render icon with dynamic routes",
"app dir - metadata dynamic routes route segment config should generate dynamic route if dynamic config is force-dynamic",
"app dir - metadata dynamic routes should generate static paths of dynamic sitemap in production",
"app dir - metadata dynamic routes should error if the default export of dynamic image is missing",
"app dir - metadata dynamic routes should error when id is missing in generateImageMetadata",
"app dir - metadata dynamic routes should error when id is missing in generateSitemaps",
"app dir - metadata dynamic routes should generate unique path for image routes under group routes",
"app dir - metadata dynamic routes should include default og font files in file trace",
"app dir - metadata dynamic routes should inject dynamic metadata properly to head",
"app dir - metadata dynamic routes should optimize routes without multiple generation API as static routes",
"app dir - metadata dynamic routes should pick configured metadataBase instead of deployment url for canonical url",
"app dir - metadata dynamic routes should support edge runtime of image routes",
"app dir - metadata dynamic routes should use localhost for local prod and fallback to deployment url when metadataBase is falsy",
"app dir - metadata dynamic routes social image routes should fill params into dynamic routes url of metadata images",
"app dir - metadata dynamic routes social image routes should fill params into routes groups url of static images",
@ -1886,13 +1884,19 @@
"app dir - metadata dynamic routes social image routes should handle manifest.[ext] dynamic routes",
"app dir - metadata dynamic routes social image routes should render og image with opengraph-image dynamic routes",
"app dir - metadata dynamic routes social image routes should render og image with twitter-image dynamic routes",
"app dir - metadata dynamic routes social image routes should support generate multi images with generateImageMetadata",
"app dir - metadata dynamic routes social image routes should support generate multi sitemaps with generateSitemaps",
"app dir - metadata dynamic routes social image routes should support params as argument in dynamic routes",
"app dir - metadata dynamic routes text routes should handle robots.[ext] dynamic routes",
"app dir - metadata dynamic routes text routes should handle sitemap.[ext] dynamic routes",
"app dir - metadata dynamic routes text routes should not throw if client components are imported but not used",
"app dir - metadata dynamic routes text routes should support alternate.languages in sitemap"
"app dir - metadata dynamic routes social image routes should support generate multi images with generateImageMetadata",
"app dir - metadata dynamic routes robots.txt should handle robots.[ext] dynamic routes",
"app dir - metadata dynamic routes should support edge runtime of image routes",
"app dir - metadata dynamic routes should include default og font files in file trace",
"app dir - metadata dynamic routes route segment config should generate dynamic route if dynamic config is force-dynamic",
"app dir - metadata dynamic routes sitemap should not throw if client components are imported but not used in sitemap",
"app dir - metadata dynamic routes sitemap should handle sitemap.[ext] dynamic routes",
"app dir - metadata dynamic routes sitemap social image routes should support generate multi sitemaps with generateSitemaps",
"app dir - metadata dynamic routes sitemap should support alternate.languages in sitemap",
"app dir - metadata dynamic routes sitemap should support generate multi sitemaps with generateSitemaps",
"app dir - metadata dynamic routes sitemap should optimize routes without multiple generation API as static routes",
"app dir - metadata dynamic routes sitemap should generate static paths of dynamic sitemap in production"
],
"pending": [],
"flakey": [],

12
test/turbopack-dev-tests-manifest.json
View file

@ -4089,13 +4089,13 @@
"app dir - metadata dynamic routes social image routes should handle manifest.[ext] dynamic routes",
"app dir - metadata dynamic routes social image routes should render og image with opengraph-image dynamic routes",
"app dir - metadata dynamic routes social image routes should render og image with twitter-image dynamic routes",
"app dir - metadata dynamic routes social image routes should support generate multi images with generateImageMetadata",
"app dir - metadata dynamic routes social image routes should support generate multi sitemaps with generateSitemaps",
"app dir - metadata dynamic routes social image routes should support params as argument in dynamic routes",
"app dir - metadata dynamic routes text routes should handle robots.[ext] dynamic routes",
"app dir - metadata dynamic routes text routes should handle sitemap.[ext] dynamic routes",
"app dir - metadata dynamic routes text routes should not throw if client components are imported but not used",
"app dir - metadata dynamic routes text routes should support alternate.languages in sitemap"
"app dir - metadata dynamic routes social image routes should support generate multi images with generateImageMetadata",
"app dir - metadata dynamic routes sitemap should not throw if client components are imported but not used in sitemap",
"app dir - metadata dynamic routes sitemap should handle sitemap.[ext] dynamic routes",
"app dir - metadata dynamic routes sitemap social image routes should support generate multi sitemaps with generateSitemaps",
"app dir - metadata dynamic routes sitemap should support alternate.languages in sitemap",
"app dir - metadata dynamic routes robots.txt should handle robots.[ext] dynamic routes"
],
"failed": [],
"pending": [],