From cfedf9c3ad32035939743b58561cba507659e98f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BF=A0=20/=20green?= Date: Tue, 13 Dec 2022 00:15:53 +0900 Subject: [PATCH] fix: preview fallback (#11312) --- package.json | 3 +- .../vite/src/node/__tests__/utils.spec.ts | 7 --- packages/vite/src/node/preview.ts | 12 +++--- .../src/node/server/middlewares/static.ts | 36 ++++++++++++---- packages/vite/src/node/utils.ts | 28 +++--------- patches/sirv@2.0.2.patch | 43 +++++++++++++++++++ playground/assets/__tests__/assets.spec.ts | 21 ++++++--- playground/assets/static/bar | 1 + pnpm-lock.yaml | 30 ++++++++++++- 9 files changed, 126 insertions(+), 55 deletions(-) create mode 100644 patches/sirv@2.0.2.patch create mode 100644 playground/assets/static/bar diff --git a/package.json b/package.json index 924c99ca8084fa..00b41c75c62b20 100644 --- a/package.json +++ b/package.json @@ -129,7 +129,8 @@ } }, "patchedDependencies": { - "dotenv-expand@9.0.0": "patches/dotenv-expand@9.0.0.patch" + "dotenv-expand@9.0.0": "patches/dotenv-expand@9.0.0.patch", + "sirv@2.0.2": "patches/sirv@2.0.2.patch" } } } diff --git a/packages/vite/src/node/__tests__/utils.spec.ts b/packages/vite/src/node/__tests__/utils.spec.ts index 073aa43da14eee..65a88ed708230b 100644 --- a/packages/vite/src/node/__tests__/utils.spec.ts +++ b/packages/vite/src/node/__tests__/utils.spec.ts @@ -12,7 +12,6 @@ import { posToNumber, processSrcSetSync, resolveHostname, - shouldServe, } from '../utils' describe('injectQuery', () => { @@ -242,12 +241,6 @@ describe('asyncFlatten', () => { }) }) -describe('shouldServe', () => { - test('returns false for malformed URLs', () => { - expect(shouldServe('/%c0%ae%c0%ae/etc/passwd', '/assets/dir')).toBe(false) - }) -}) - describe('isFileReadable', () => { test("file doesn't exist", async () => { expect(isFileReadable('/does_not_exist')).toBe(false) diff --git a/packages/vite/src/node/preview.ts b/packages/vite/src/node/preview.ts index 3a5cbc0c8e566c..dd8180faa7dc3e 100644 --- a/packages/vite/src/node/preview.ts +++ b/packages/vite/src/node/preview.ts @@ -15,7 +15,7 @@ import { import { openBrowser } from './server/openBrowser' import compression from './server/middlewares/compression' import { proxyMiddleware } from './server/middlewares/proxy' -import { resolveHostname, resolveServerUrls, shouldServe } from './utils' +import { resolveHostname, resolveServerUrls, shouldServeFile } from './utils' import { printServerUrls } from './logger' import { resolveConfig } from '.' import type { InlineConfig, ResolvedConfig } from '.' @@ -128,13 +128,11 @@ export async function preview( } } }, + shouldServe(filePath) { + return shouldServeFile(filePath, distDir) + }, }) - app.use(previewBase, async (req, res, next) => { - if (shouldServe(req.url!, distDir)) { - return assetServer(req, res, next) - } - next() - }) + app.use(previewBase, assetServer) // apply post server hooks from plugins postHooks.forEach((fn) => fn && fn()) diff --git a/packages/vite/src/node/server/middlewares/static.ts b/packages/vite/src/node/server/middlewares/static.ts index 4e54af75d84538..229b860dbcec56 100644 --- a/packages/vite/src/node/server/middlewares/static.ts +++ b/packages/vite/src/node/server/middlewares/static.ts @@ -14,11 +14,17 @@ import { isInternalRequest, isParentDirectory, isWindows, - shouldServe, + shouldServeFile, slash, } from '../../utils' -const sirvOptions = (headers?: OutgoingHttpHeaders): Options => { +const sirvOptions = ({ + headers, + shouldServe, +}: { + headers?: OutgoingHttpHeaders + shouldServe?: (p: string) => void +}): Options => { return { dev: true, etag: true, @@ -38,6 +44,7 @@ const sirvOptions = (headers?: OutgoingHttpHeaders): Options => { } } }, + shouldServe, } } @@ -45,7 +52,13 @@ export function servePublicMiddleware( dir: string, headers?: OutgoingHttpHeaders, ): Connect.NextHandleFunction { - const serve = sirv(dir, sirvOptions(headers)) + const serve = sirv( + dir, + sirvOptions({ + headers, + shouldServe: (filePath) => shouldServeFile(filePath, dir), + }), + ) // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...` return function viteServePublicMiddleware(req, res, next) { @@ -53,10 +66,7 @@ export function servePublicMiddleware( if (isImportRequest(req.url!) || isInternalRequest(req.url!)) { return next() } - if (shouldServe(req.url!, dir)) { - return serve(req, res, next) - } - next() + serve(req, res, next) } } @@ -64,7 +74,12 @@ export function serveStaticMiddleware( dir: string, server: ViteDevServer, ): Connect.NextHandleFunction { - const serve = sirv(dir, sirvOptions(server.config.server.headers)) + const serve = sirv( + dir, + sirvOptions({ + headers: server.config.server.headers, + }), + ) // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...` return function viteServeStaticMiddleware(req, res, next) { @@ -124,7 +139,10 @@ export function serveStaticMiddleware( export function serveRawFsMiddleware( server: ViteDevServer, ): Connect.NextHandleFunction { - const serveFromRoot = sirv('/', sirvOptions(server.config.server.headers)) + const serveFromRoot = sirv( + '/', + sirvOptions({ headers: server.config.server.headers }), + ) // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...` return function viteServeRawFsMiddleware(req, res, next) { diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index b78d003a6001ed..d56b2fd5b8e302 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -1196,29 +1196,11 @@ export const isNonDriveRelativeAbsolutePath = (p: string): boolean => { * Determine if a file is being requested with the correct case, to ensure * consistent behaviour between dev and prod and across operating systems. */ -export function shouldServe(url: string, assetsDir: string): boolean { - try { - // viteTestUrl is set to something like http://localhost:4173/ and then many tests make calls - // like `await page.goto(viteTestUrl + '/example')` giving us URLs beginning with a double slash - const pathname = decodeURI( - new URL( - url.startsWith('//') ? url.substring(1) : url, - 'http://example.com', - ).pathname, - ) - const file = path.join(assetsDir, pathname) - if ( - !fs.existsSync(file) || - (isCaseInsensitiveFS && // can skip case check on Linux - !fs.statSync(file).isDirectory() && - !hasCorrectCase(file, assetsDir)) - ) { - return false - } - return true - } catch (err) { - return false - } +export function shouldServeFile(filePath: string, root: string): boolean { + // can skip case check on Linux + if (!isCaseInsensitiveFS) return true + + return hasCorrectCase(filePath, root) } /** diff --git a/patches/sirv@2.0.2.patch b/patches/sirv@2.0.2.patch new file mode 100644 index 00000000000000..04a604eec2926e --- /dev/null +++ b/patches/sirv@2.0.2.patch @@ -0,0 +1,43 @@ +diff --git a/build.mjs b/build.mjs +index fe01068d0dd3f788a0978802db1747dd83c5825e..fb099c38cc2cbd59300471e7307625e2fc127f0c 100644 +--- a/build.mjs ++++ b/build.mjs +@@ -35,7 +35,7 @@ function viaCache(cache, uri, extns) { + } + } + +-function viaLocal(dir, isEtag, uri, extns) { ++function viaLocal(dir, isEtag, uri, extns, shouldServe) { + let i=0, arr=toAssume(uri, extns); + let abs, stats, name, headers; + for (; i < arr.length; i++) { +@@ -43,6 +43,7 @@ function viaLocal(dir, isEtag, uri, extns) { + if (abs.startsWith(dir) && fs.existsSync(abs)) { + stats = fs.statSync(abs); + if (stats.isDirectory()) continue; ++ if (shouldServe && !shouldServe(abs)) continue; + headers = toHeaders(name, stats, isEtag); + headers['Cache-Control'] = isEtag ? 'no-cache' : 'no-store'; + return { abs, stats, headers }; +@@ -172,7 +173,7 @@ export default function (dir, opts={}) { + catch (err) { /* malform uri */ } + } + +- let data = lookup(pathname, extns) || isSPA && !isMatch(pathname, ignores) && lookup(fallback, extns); ++ let data = lookup(pathname, extns, opts.shouldServe) || isSPA && !isMatch(pathname, ignores) && lookup(fallback, extns, opts.shouldServe); + if (!data) return next ? next() : isNotFound(req, res); + + if (isEtag && req.headers['if-none-match'] === data.headers['ETag']) { +diff --git a/sirv.d.ts b/sirv.d.ts +index c05040fc6ec504a1828a7badd39f669981acd0ee..e9597e8b5bf24613a09565f0e13024ae3ca8fa5e 100644 +--- a/sirv.d.ts ++++ b/sirv.d.ts +@@ -19,6 +19,8 @@ declare module 'sirv' { + gzip?: boolean; + onNoMatch?: (req: IncomingMessage, res: ServerResponse) => void; + setHeaders?: (res: ServerResponse, pathname: string, stats: Stats) => void; ++ /** patched */ ++ shouldServe?: (absoluteFilePath: string) => void; + } + + export default function(dir?: string, opts?: Options): RequestHandler; \ No newline at end of file diff --git a/playground/assets/__tests__/assets.spec.ts b/playground/assets/__tests__/assets.spec.ts index 482cf5a3b8a599..433dc161352b98 100644 --- a/playground/assets/__tests__/assets.spec.ts +++ b/playground/assets/__tests__/assets.spec.ts @@ -24,6 +24,12 @@ const assetMatch = isBuild const iconMatch = `/foo/icon.png` +const fetchPath = (p: string) => { + return fetch(path.posix.join(viteTestUrl, p), { + headers: { Accept: 'text/html,*/*' }, + }) +} + test('should have no 404s', () => { browserLogs.forEach((msg) => { expect(msg).not.toMatch('404') @@ -31,12 +37,15 @@ test('should have no 404s', () => { }) test('should get a 404 when using incorrect case', async () => { - expect((await fetch(path.posix.join(viteTestUrl, 'icon.png'))).status).toBe( - 200, - ) - expect((await fetch(path.posix.join(viteTestUrl, 'ICON.png'))).status).toBe( - 404, - ) + expect((await fetchPath('icon.png')).status).toBe(200) + // won't be wrote to index.html because the url includes `.` + expect((await fetchPath('ICON.png')).status).toBe(404) + + expect((await fetchPath('bar')).status).toBe(200) + // fallback to index.html + const incorrectBarFetch = await fetchPath('BAR') + expect(incorrectBarFetch.status).toBe(200) + expect(incorrectBarFetch.headers.get('Content-Type')).toContain('text/html') }) describe('injected scripts', () => { diff --git a/playground/assets/static/bar b/playground/assets/static/bar new file mode 100644 index 00000000000000..5716ca5987cbf9 --- /dev/null +++ b/playground/assets/static/bar @@ -0,0 +1 @@ +bar diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 14666852ed1f0b..589ad1ed077a84 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -6,6 +6,9 @@ overrides: packageExtensionsChecksum: 2a87e01b470616d3b7def19cc0830231 patchedDependencies: + sirv@2.0.2: + hash: w6q35pvk7bmykgqf2hieut43iq + path: patches/sirv@2.0.2.patch dotenv-expand@9.0.0: hash: 6vdpvodnfhahap67xag6diqqtu path: patches/dotenv-expand@9.0.0.patch @@ -274,7 +277,7 @@ importers: postcss-load-config: 4.0.1_postcss@8.4.20 postcss-modules: 6.0.0_postcss@8.4.20 resolve.exports: 1.1.0 - sirv: 2.0.2 + sirv: 2.0.2_w6q35pvk7bmykgqf2hieut43iq source-map-js: 1.0.2 source-map-support: 0.5.21 strip-ansi: 7.0.1 @@ -4707,6 +4710,19 @@ packages: peerDependenciesMeta: debug: optional: true + dev: false + + /follow-redirects/1.15.0_debug@4.3.4: + resolution: {integrity: sha512-aExlJShTV4qOUOL7yF1U5tvLCB0xQuudbf6toyYA0E/acBNw71mvjFTnLaRp50aQaYocMR0a/RMMBIHeZnGyjQ==} + engines: {node: '>=4.0'} + peerDependencies: + debug: '*' + peerDependenciesMeta: + debug: + optional: true + dependencies: + debug: 4.3.4 + dev: true /form-data/4.0.0: resolution: {integrity: sha512-ETEklSGi5t0QMZuiXoA/Q6vcnxcLQP5vdugSpuAyi6SVGi2clPPp+xgEhuMaHC+zGgn31Kd235W35f7Hykkaww==} @@ -5097,7 +5113,7 @@ packages: engines: {node: '>=8.0.0'} dependencies: eventemitter3: 4.0.7 - follow-redirects: 1.15.0 + follow-redirects: 1.15.0_debug@4.3.4 requires-port: 1.0.0 transitivePeerDependencies: - debug @@ -7386,6 +7402,16 @@ packages: totalist: 3.0.0 dev: true + /sirv/2.0.2_w6q35pvk7bmykgqf2hieut43iq: + resolution: {integrity: sha512-4Qog6aE29nIjAOKe/wowFTxOdmbEZKb+3tsLljaBRzJwtqto0BChD2zzH0LhgCSXiI+V7X+Y45v14wBZQ1TK3w==} + engines: {node: '>= 10'} + dependencies: + '@polka/url': 1.0.0-next.21 + mrmime: 1.0.1 + totalist: 3.0.0 + dev: true + patched: true + /sisteransi/1.0.5: resolution: {integrity: sha512-bLGGlR1QxBcynn2d5YmDX4MGjlZvy2MRBDRNHLJ8VI6l6+9FUiyTFNJ0IveOSP0bcXgVDPRcfGqA0pjaqUpfVg==} dev: true