Skip to content

Commit

Permalink
Fix metadata json manifest convention (#62615)
Browse files Browse the repository at this point in the history
### What

Change from processing the file with `next-metatdata-route-loader`
directly into passing the file as loader query, and leave an empty
resource file for it. This will resolve the error that users were seeing
with `manifest.json` convention.

```
Import trace for requested module:
../../../../packages/next/dist/build/webpack/loaders/next-metadata-route-loader.js?page=%2Fmanifest.jso
n%2Froute&isDynamic=0!./app/manifest.json?__next_metadata_route__
getStaticAssetRouteCode page /manifest.json/route this.resourcePath /Users/huozhi/workspace/next.js/tes
t/e2e/app-dir/metadata-json-manifest/app/manifest.json
```

### Why

I looked at the loader process that the final resource processed by
webpack is `json!next-metadata-route-loader...`, which means the builtin
json loader processing json file after the metadata route loader. I
didn't get chance to solve the ordering issue, so I changed the
resourcePath to empty "", and pass the file path as query into the
loader to avoid json-loader processing it after transpilation.


Fixes #59923

Closes NEXT-2630
Closes NEXT-2439
  • Loading branch information
huozhi committed Feb 27, 2024
1 parent b0fcd44 commit 69d1edf
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 10 deletions.
3 changes: 2 additions & 1 deletion packages/next/src/build/webpack/loaders/next-app-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,9 @@ async function createAppRouteCode({

resolvedPagePath = `next-metadata-route-loader?${stringify({
page,
filePath: resolvedPagePath,
isDynamic: isDynamic ? '1' : '0',
})}!${resolvedPagePath}${`?${WEBPACK_RESOURCE_QUERIES.metadataRoute}`}`
})}!?${WEBPACK_RESOURCE_QUERIES.metadataRoute}`
}

const pathname = new AppPathnameNormalizer().normalize(page)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const cacheHeader = {

type MetadataRouteLoaderOptions = {
page: string
filePath: string
isDynamic: '1' | '0'
}

Expand All @@ -46,7 +47,6 @@ function getContentType(resourcePath: string) {
return 'text/plain'
}

// Strip metadata resource query string from `import.meta.url` to make sure the fs.readFileSync get the right path.
async function getStaticAssetRouteCode(
resourcePath: string,
fileBaseName: string
Expand All @@ -58,6 +58,7 @@ async function getStaticAssetRouteCode(
? cacheHeader.none
: cacheHeader.longCache
const code = `\
/* static asset route */
import { NextResponse } from 'next/server'
const contentType = ${JSON.stringify(getContentType(resourcePath))}
Expand All @@ -82,6 +83,7 @@ export const dynamic = 'force-static'

function getDynamicTextRouteCode(resourcePath: string) {
return `\
/* dynamic asset route */
import { NextResponse } from 'next/server'
import handler from ${JSON.stringify(resourcePath)}
import { resolveRouteData } from 'next/dist/build/webpack/loaders/metadata/resolve-route-data'
Expand All @@ -108,6 +110,7 @@ export async function GET() {
// <metadata-image>/[id]/route.js
function getDynamicImageRouteCode(resourcePath: string) {
return `\
/* dynamic image route */
import { NextResponse } from 'next/server'
import * as userland from ${JSON.stringify(resourcePath)}
Expand Down Expand Up @@ -159,6 +162,7 @@ async function getDynamicSiteMapRouteCode(
page.includes('[__metadata_id__]')
) {
staticGenerationCode = `\
/* dynamic sitemap route */
export async function generateStaticParams() {
const sitemaps = generateSitemaps ? await generateSitemaps() : []
const params = []
Expand Down Expand Up @@ -232,26 +236,25 @@ ${staticGenerationCode}
`
return code
}
// `import.meta.url` is the resource name of the current module.

// When it's static route, it could be favicon.ico, sitemap.xml, robots.txt etc.
// TODO-METADATA: improve the cache control strategy
const nextMetadataRouterLoader: webpack.LoaderDefinitionFunction<MetadataRouteLoaderOptions> =
async function () {
const { resourcePath } = this
const { page, isDynamic } = this.getOptions()
const { name: fileBaseName } = getFilenameAndExtension(resourcePath)
const { page, isDynamic, filePath } = this.getOptions()
const { name: fileBaseName } = getFilenameAndExtension(filePath)

let code = ''
if (isDynamic === '1') {
if (fileBaseName === 'robots' || fileBaseName === 'manifest') {
code = getDynamicTextRouteCode(resourcePath)
code = getDynamicTextRouteCode(filePath)
} else if (fileBaseName === 'sitemap') {
code = await getDynamicSiteMapRouteCode(resourcePath, page, this)
code = await getDynamicSiteMapRouteCode(filePath, page, this)
} else {
code = getDynamicImageRouteCode(resourcePath)
code = getDynamicImageRouteCode(filePath)
}
} else {
code = await getStaticAssetRouteCode(resourcePath, fileBaseName)
code = await getStaticAssetRouteCode(filePath, fileBaseName)
}

return code
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/app-dir/metadata-json-manifest/app/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export const metadata = {
title: 'Next.js',
description: 'Generated by Next.js',
}

export default function RootLayout({ children }) {
return (
<html lang="en">
<body>{children}</body>
</html>
)
}
6 changes: 6 additions & 0 deletions test/e2e/app-dir/metadata-json-manifest/app/manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "My Next.js Application",
"short_name": "Next.js App",
"description": "An application built with Next.js",
"start_url": "/"
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/metadata-json-manifest/app/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function page() {
return 'page.js'
}
22 changes: 22 additions & 0 deletions test/e2e/app-dir/metadata-json-manifest/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { createNextDescribe } from 'e2e-utils'

createNextDescribe(
'app-dir metadata-json-manifest',
{
files: __dirname,
skipDeployment: true,
},
({ next }) => {
it('should support metadata.json manifest', async () => {
const response = await next.fetch('/manifest.json')
expect(response.status).toBe(200)
const json = await response.json()
expect(json).toEqual({
name: 'My Next.js Application',
short_name: 'Next.js App',
description: 'An application built with Next.js',
start_url: '/',
})
})
}
)

0 comments on commit 69d1edf

Please sign in to comment.