Escape url-unsafe characters in names of app router scripts and styles (#64131)
Building on #58293, this expands escaping of url-unsafe characters in paths to “required” scripts and styles in the app renderer. This also refactors the test introduced in #58293 and expands it to include stylesheet references as well as checking resources in the head, which include special characters like turbopack references like `[turbopack]`. Test Plan: `TURBOPACK=1 pnpm test-dev test/e2e/app-dir/resource-url-encoding` Closes PACK-2911
This commit is contained in:
parent
02dd1e55f0
commit
11575a45da
11 changed files with 71 additions and 45 deletions
|
@ -3,6 +3,7 @@ import { getLinkAndScriptTags } from './get-css-inlined-link-tags'
|
|||
import { getPreloadableFonts } from './get-preloadable-fonts'
|
||||
import type { AppRenderContext } from './app-render'
|
||||
import { getAssetQueryString } from './get-asset-query-string'
|
||||
import { encodeURIPath } from '../../shared/lib/encode-uri-path'
|
||||
|
||||
export function getLayerAssets({
|
||||
ctx,
|
||||
|
@ -41,7 +42,7 @@ export function getLayerAssets({
|
|||
const fontFilename = preloadedFontFiles[i]
|
||||
const ext = /\.(woff|woff2|eot|ttf|otf)$/.exec(fontFilename)![1]
|
||||
const type = `font/${ext}`
|
||||
const href = `${ctx.assetPrefix}/_next/${fontFilename}`
|
||||
const href = `${ctx.assetPrefix}/_next/${encodeURIPath(fontFilename)}`
|
||||
ctx.componentMod.preloadFont(href, type, ctx.renderOpts.crossOrigin)
|
||||
}
|
||||
} else {
|
||||
|
@ -64,10 +65,9 @@ export function getLayerAssets({
|
|||
// Because of this, we add a `?v=` query to bypass the cache during
|
||||
// development. We need to also make sure that the number is always
|
||||
// increasing.
|
||||
const fullHref = `${ctx.assetPrefix}/_next/${href}${getAssetQueryString(
|
||||
ctx,
|
||||
true
|
||||
)}`
|
||||
const fullHref = `${ctx.assetPrefix}/_next/${encodeURIPath(
|
||||
href
|
||||
)}${getAssetQueryString(ctx, true)}`
|
||||
|
||||
// `Precedence` is an opt-in signal for React to handle resource
|
||||
// loading and deduplication, etc. It's also used as the key to sort
|
||||
|
@ -95,10 +95,9 @@ export function getLayerAssets({
|
|||
|
||||
const scripts = scriptTags
|
||||
? scriptTags.map((href, index) => {
|
||||
const fullSrc = `${ctx.assetPrefix}/_next/${href}${getAssetQueryString(
|
||||
ctx,
|
||||
true
|
||||
)}`
|
||||
const fullSrc = `${ctx.assetPrefix}/_next/${encodeURIPath(
|
||||
href
|
||||
)}${getAssetQueryString(ctx, true)}`
|
||||
|
||||
return <script src={fullSrc} async={true} key={`script-${index}`} />
|
||||
})
|
||||
|
|
|
@ -1,3 +1,4 @@
|
|||
import { encodeURIPath } from '../../shared/lib/encode-uri-path'
|
||||
import type { BuildManifest } from '../get-page-files'
|
||||
|
||||
import ReactDOM from 'react-dom'
|
||||
|
@ -24,7 +25,7 @@ export function getRequiredScripts(
|
|||
crossOrigin,
|
||||
}
|
||||
|
||||
const files = buildManifest.rootMainFiles
|
||||
const files = buildManifest.rootMainFiles.map(encodeURIPath)
|
||||
if (files.length === 0) {
|
||||
throw new Error(
|
||||
'Invariant: missing bootstrap script. This is a bug in Next.js'
|
||||
|
|
|
@ -558,7 +558,7 @@ export async function setupFsCheck(opts: {
|
|||
|
||||
// check decoded variant as well
|
||||
if (!matchedItem && !opts.dev) {
|
||||
matchedItem = items.has(curItemPath)
|
||||
matchedItem = items.has(curDecodedItemPath)
|
||||
if (matchedItem) curItemPath = curDecodedItemPath
|
||||
else {
|
||||
// x-ref: https://github.com/vercel/next.js/issues/54008
|
||||
|
|
|
@ -1,4 +1,5 @@
|
|||
import Compoment from './client#component'
|
||||
import '../my@style.css'
|
||||
|
||||
export default function Page() {
|
||||
return <Compoment />
|
3
test/e2e/app-dir/resource-url-encoding/my@style.css
Normal file
3
test/e2e/app-dir/resource-url-encoding/my@style.css
Normal file
|
@ -0,0 +1,3 @@
|
|||
body {
|
||||
background: rgb(0, 0, 255);
|
||||
}
|
|
@ -1,4 +1,5 @@
|
|||
import dynamic from 'next/dynamic'
|
||||
import '../my@style.css'
|
||||
|
||||
const Component = dynamic(() => import('../app/client#component'))
|
||||
|
|
@ -0,0 +1,55 @@
|
|||
/* eslint-disable jest/no-standalone-expect */
|
||||
import { nextTestSetup } from 'e2e-utils'
|
||||
|
||||
describe('scripts', () => {
|
||||
const { next } = nextTestSetup({
|
||||
files: __dirname,
|
||||
})
|
||||
|
||||
// TODO: fix test case in webpack
|
||||
// It's failing with `Could not find the module ".../app/client#component.tsx#" in the React Client Manifest. This is probably a bug in the React Server Components bundler.`
|
||||
;(process.env.TURBOPACK ? it : it.skip).each(['app', 'pages'])(
|
||||
'encodes characters in %s router',
|
||||
async (routerType) => {
|
||||
const browser = await next.browser(routerType === 'app' ? '/' : '/pages')
|
||||
expect(await browser.elementByCss('p').text()).toBe('hello world')
|
||||
const scripts = await browser.elementsByCss('script')
|
||||
expect(scripts.length).toBeGreaterThan(0)
|
||||
for (const script of scripts) {
|
||||
const src = await script.evaluate((script) => script.src)
|
||||
expect(src).not.toContain('#')
|
||||
expect(src).not.toContain('[')
|
||||
}
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
describe('styles', () => {
|
||||
const { next } = nextTestSetup({
|
||||
files: __dirname,
|
||||
})
|
||||
// TODO: fix test case in webpack
|
||||
// It's failing with `Could not find the module ".../app/client#component.tsx#" in the React Client Manifest. This is probably a bug in the React Server Components bundler.`
|
||||
;(process.env.TURBOPACK ? it : it.skip).each(['app', 'pages'])(
|
||||
'encodes characters in %s router',
|
||||
async (routerType) => {
|
||||
const browser = await next.browser(routerType === 'app' ? '/' : '/pages')
|
||||
|
||||
let body = await browser.elementByCss('body')
|
||||
expect(
|
||||
await body.evaluate((el) =>
|
||||
getComputedStyle(el).getPropertyValue('background-color')
|
||||
)
|
||||
).toBe('rgb(0, 0, 255)')
|
||||
|
||||
const stylesheets = await browser.elementsByCss('link[rel="stylesheet"]')
|
||||
expect(stylesheets.length).toBeGreaterThan(0)
|
||||
for (const stylesheet of stylesheets) {
|
||||
const href = await stylesheet.evaluate((stylesheet) => stylesheet.href)
|
||||
console.log('app href', href)
|
||||
expect(href).not.toContain('#')
|
||||
expect(href).not.toContain('[')
|
||||
}
|
||||
}
|
||||
)
|
||||
})
|
|
@ -1,34 +0,0 @@
|
|||
/* eslint-disable jest/no-standalone-expect */
|
||||
import { nextTestSetup } from 'e2e-utils'
|
||||
|
||||
describe('weird chars in scripts', () => {
|
||||
const { next } = nextTestSetup({
|
||||
files: __dirname,
|
||||
})
|
||||
// TODO: fix test case in webpack
|
||||
// It's failing with `Could not find the module ".../app/client#component.tsx#" in the React Client Manifest. This is probably a bug in the React Server Components bundler.`
|
||||
;(process.env.TURBOPACK ? it : it.skip)(
|
||||
'should load in the browser',
|
||||
async () => {
|
||||
const browser = await next.browser('/')
|
||||
expect(await browser.elementByCss('p').text()).toBe('hello world')
|
||||
const scripts = await browser.elementsByCss('script')
|
||||
for (const script of scripts) {
|
||||
const src = await script.evaluate((script) => script.src)
|
||||
expect(src).not.toContain('#')
|
||||
}
|
||||
}
|
||||
)
|
||||
;(process.env.TURBOPACK ? it : it.skip)(
|
||||
'should load in the browser',
|
||||
async () => {
|
||||
const browser = await next.browser('/pages')
|
||||
expect(await browser.elementByCss('p').text()).toBe('hello world')
|
||||
const scripts = await browser.elementsByCss('script')
|
||||
for (const script of scripts) {
|
||||
const src = await script.evaluate((script) => script.src)
|
||||
expect(src).not.toContain('#')
|
||||
}
|
||||
}
|
||||
)
|
||||
})
|
Loading…
Reference in a new issue