Skip to content

Commit cfedf9c

Browse files
sapphi-redsapphi-red
authored
fix: preview fallback (#11312)
1 parent d0a9281 commit cfedf9c

File tree

9 files changed

+126
-55
lines changed

9 files changed

+126
-55
lines changed

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@
129129
}
130130
},
131131
"patchedDependencies": {
132-
"dotenv-expand@9.0.0": "patches/dotenv-expand@9.0.0.patch"
132+
"dotenv-expand@9.0.0": "patches/dotenv-expand@9.0.0.patch",
133+
"sirv@2.0.2": "patches/sirv@2.0.2.patch"
133134
}
134135
}
135136
}

‎packages/vite/src/node/__tests__/utils.spec.ts‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
posToNumber,
1313
processSrcSetSync,
1414
resolveHostname,
15-
shouldServe,
1615
} from '../utils'
1716

1817
describe('injectQuery', () => {
@@ -242,12 +241,6 @@ describe('asyncFlatten', () => {
242241
})
243242
})
244243

245-
describe('shouldServe', () => {
246-
test('returns false for malformed URLs', () => {
247-
expect(shouldServe('/%c0%ae%c0%ae/etc/passwd', '/assets/dir')).toBe(false)
248-
})
249-
})
250-
251244
describe('isFileReadable', () => {
252245
test("file doesn't exist", async () => {
253246
expect(isFileReadable('/does_not_exist')).toBe(false)

‎packages/vite/src/node/preview.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
import { openBrowser } from './server/openBrowser'
1616
import compression from './server/middlewares/compression'
1717
import { proxyMiddleware } from './server/middlewares/proxy'
18-
import { resolveHostname, resolveServerUrls, shouldServe } from './utils'
18+
import { resolveHostname, resolveServerUrls, shouldServeFile } from './utils'
1919
import { printServerUrls } from './logger'
2020
import { resolveConfig } from '.'
2121
import type { InlineConfig, ResolvedConfig } from '.'
@@ -128,13 +128,11 @@ export async function preview(
128128
}
129129
}
130130
},
131+
shouldServe(filePath) {
132+
return shouldServeFile(filePath, distDir)
133+
},
131134
})
132-
app.use(previewBase, async (req, res, next) => {
133-
if (shouldServe(req.url!, distDir)) {
134-
return assetServer(req, res, next)
135-
}
136-
next()
137-
})
135+
app.use(previewBase, assetServer)
138136

139137
// apply post server hooks from plugins
140138
postHooks.forEach((fn) => fn && fn())

‎packages/vite/src/node/server/middlewares/static.ts‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,17 @@ import {
1414
isInternalRequest,
1515
isParentDirectory,
1616
isWindows,
17-
shouldServe,
17+
shouldServeFile,
1818
slash,
1919
} from '../../utils'
2020

21-
const sirvOptions = (headers?: OutgoingHttpHeaders): Options => {
21+
const sirvOptions = ({
22+
headers,
23+
shouldServe,
24+
}: {
25+
headers?: OutgoingHttpHeaders
26+
shouldServe?: (p: string) => void
27+
}): Options => {
2228
return {
2329
dev: true,
2430
etag: true,
@@ -38,33 +44,42 @@ const sirvOptions = (headers?: OutgoingHttpHeaders): Options => {
3844
}
3945
}
4046
},
47+
shouldServe,
4148
}
4249
}
4350

4451
export function servePublicMiddleware(
4552
dir: string,
4653
headers?: OutgoingHttpHeaders,
4754
): Connect.NextHandleFunction {
48-
const serve = sirv(dir, sirvOptions(headers))
55+
const serve = sirv(
56+
dir,
57+
sirvOptions({
58+
headers,
59+
shouldServe: (filePath) => shouldServeFile(filePath, dir),
60+
}),
61+
)
4962

5063
// Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
5164
return function viteServePublicMiddleware(req, res, next) {
5265
// skip import request and internal requests `/@fs/ /@vite-client` etc...
5366
if (isImportRequest(req.url!) || isInternalRequest(req.url!)) {
5467
return next()
5568
}
56-
if (shouldServe(req.url!, dir)) {
57-
return serve(req, res, next)
58-
}
59-
next()
69+
serve(req, res, next)
6070
}
6171
}
6272

6373
export function serveStaticMiddleware(
6474
dir: string,
6575
server: ViteDevServer,
6676
): Connect.NextHandleFunction {
67-
const serve = sirv(dir, sirvOptions(server.config.server.headers))
77+
const serve = sirv(
78+
dir,
79+
sirvOptions({
80+
headers: server.config.server.headers,
81+
}),
82+
)
6883

6984
// Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
7085
return function viteServeStaticMiddleware(req, res, next) {
@@ -124,7 +139,10 @@ export function serveStaticMiddleware(
124139
export function serveRawFsMiddleware(
125140
server: ViteDevServer,
126141
): Connect.NextHandleFunction {
127-
const serveFromRoot = sirv('/', sirvOptions(server.config.server.headers))
142+
const serveFromRoot = sirv(
143+
'/',
144+
sirvOptions({ headers: server.config.server.headers }),
145+
)
128146

129147
// Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
130148
return function viteServeRawFsMiddleware(req, res, next) {

‎packages/vite/src/node/utils.ts‎

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,29 +1196,11 @@ export const isNonDriveRelativeAbsolutePath = (p: string): boolean => {
11961196
* Determine if a file is being requested with the correct case, to ensure
11971197
* consistent behaviour between dev and prod and across operating systems.
11981198
*/
1199-
export function shouldServe(url: string, assetsDir: string): boolean {
1200-
try {
1201-
// viteTestUrl is set to something like http://localhost:4173/ and then many tests make calls
1202-
// like `await page.goto(viteTestUrl + '/example')` giving us URLs beginning with a double slash
1203-
const pathname = decodeURI(
1204-
new URL(
1205-
url.startsWith('//') ? url.substring(1) : url,
1206-
'http://example.com',
1207-
).pathname,
1208-
)
1209-
const file = path.join(assetsDir, pathname)
1210-
if (
1211-
!fs.existsSync(file) ||
1212-
(isCaseInsensitiveFS && // can skip case check on Linux
1213-
!fs.statSync(file).isDirectory() &&
1214-
!hasCorrectCase(file, assetsDir))
1215-
) {
1216-
return false
1217-
}
1218-
return true
1219-
} catch (err) {
1220-
return false
1221-
}
1199+
export function shouldServeFile(filePath: string, root: string): boolean {
1200+
// can skip case check on Linux
1201+
if (!isCaseInsensitiveFS) return true
1202+
1203+
return hasCorrectCase(filePath, root)
12221204
}
12231205

12241206
/**

‎patches/sirv@2.0.2.patch‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
diff --git a/build.mjs b/build.mjs
2+
index fe01068d0dd3f788a0978802db1747dd83c5825e..fb099c38cc2cbd59300471e7307625e2fc127f0c 100644
3+
--- a/build.mjs
4+
+++ b/build.mjs
5+
@@ -35,7 +35,7 @@ function viaCache(cache, uri, extns) {
6+
}
7+
}
8+
9+
-function viaLocal(dir, isEtag, uri, extns) {
10+
+function viaLocal(dir, isEtag, uri, extns, shouldServe) {
11+
let i=0, arr=toAssume(uri, extns);
12+
let abs, stats, name, headers;
13+
for (; i < arr.length; i++) {
14+
@@ -43,6 +43,7 @@ function viaLocal(dir, isEtag, uri, extns) {
15+
if (abs.startsWith(dir) && fs.existsSync(abs)) {
16+
stats = fs.statSync(abs);
17+
if (stats.isDirectory()) continue;
18+
+ if (shouldServe && !shouldServe(abs)) continue;
19+
headers = toHeaders(name, stats, isEtag);
20+
headers['Cache-Control'] = isEtag ? 'no-cache' : 'no-store';
21+
return { abs, stats, headers };
22+
@@ -172,7 +173,7 @@ export default function (dir, opts={}) {
23+
catch (err) { /* malform uri */ }
24+
}
25+
26+
- let data = lookup(pathname, extns) || isSPA && !isMatch(pathname, ignores) && lookup(fallback, extns);
27+
+ let data = lookup(pathname, extns, opts.shouldServe) || isSPA && !isMatch(pathname, ignores) && lookup(fallback, extns, opts.shouldServe);
28+
if (!data) return next ? next() : isNotFound(req, res);
29+
30+
if (isEtag && req.headers['if-none-match'] === data.headers['ETag']) {
31+
diff --git a/sirv.d.ts b/sirv.d.ts
32+
index c05040fc6ec504a1828a7badd39f669981acd0ee..e9597e8b5bf24613a09565f0e13024ae3ca8fa5e 100644
33+
--- a/sirv.d.ts
34+
+++ b/sirv.d.ts
35+
@@ -19,6 +19,8 @@ declare module 'sirv' {
36+
gzip?: boolean;
37+
onNoMatch?: (req: IncomingMessage, res: ServerResponse) => void;
38+
setHeaders?: (res: ServerResponse, pathname: string, stats: Stats) => void;
39+
+ /** patched */
40+
+ shouldServe?: (absoluteFilePath: string) => void;
41+
}
42+
43+
export default function(dir?: string, opts?: Options): RequestHandler;

‎playground/assets/__tests__/assets.spec.ts‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,28 @@ const assetMatch = isBuild
2424

2525
const iconMatch = `/foo/icon.png`
2626

27+
const fetchPath = (p: string) => {
28+
return fetch(path.posix.join(viteTestUrl, p), {
29+
headers: { Accept: 'text/html,*/*' },
30+
})
31+
}
32+
2733
test('should have no 404s', () => {
2834
browserLogs.forEach((msg) => {
2935
expect(msg).not.toMatch('404')
3036
})
3137
})
3238

3339
test('should get a 404 when using incorrect case', async () => {
34-
expect((await fetch(path.posix.join(viteTestUrl, 'icon.png'))).status).toBe(
35-
200,
36-
)
37-
expect((await fetch(path.posix.join(viteTestUrl, 'ICON.png'))).status).toBe(
38-
404,
39-
)
40+
expect((await fetchPath('icon.png')).status).toBe(200)
41+
// won't be wrote to index.html because the url includes `.`
42+
expect((await fetchPath('ICON.png')).status).toBe(404)
43+
44+
expect((await fetchPath('bar')).status).toBe(200)
45+
// fallback to index.html
46+
const incorrectBarFetch = await fetchPath('BAR')
47+
expect(incorrectBarFetch.status).toBe(200)
48+
expect(incorrectBarFetch.headers.get('Content-Type')).toContain('text/html')
4049
})
4150

4251
describe('injected scripts', () => {

‎playground/assets/static/bar‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
bar

‎pnpm-lock.yaml‎

Lines changed: 28 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)