Skip to content

Commit

Permalink
fix: preview fallback (#11312)
Browse files Browse the repository at this point in the history
  • Loading branch information
sapphi-red authored Dec 12, 2022
1 parent d0a9281 commit cfedf9c
Show file tree
Hide file tree
Showing 9 changed files with 126 additions and 55 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
7 changes: 0 additions & 7 deletions packages/vite/src/node/__tests__/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
posToNumber,
processSrcSetSync,
resolveHostname,
shouldServe,
} from '../utils'

describe('injectQuery', () => {
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 5 additions & 7 deletions packages/vite/src/node/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 '.'
Expand Down Expand Up @@ -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())
Expand Down
36 changes: 27 additions & 9 deletions packages/vite/src/node/server/middlewares/static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -38,33 +44,42 @@ const sirvOptions = (headers?: OutgoingHttpHeaders): Options => {
}
}
},
shouldServe,
}
}

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) {
// skip import request and internal requests `/@fs/ /@vite-client` etc...
if (isImportRequest(req.url!) || isInternalRequest(req.url!)) {
return next()
}
if (shouldServe(req.url!, dir)) {
return serve(req, res, next)
}
next()
serve(req, res, next)
}
}

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) {
Expand Down Expand Up @@ -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) {
Expand Down
28 changes: 5 additions & 23 deletions packages/vite/src/node/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

/**
Expand Down
43 changes: 43 additions & 0 deletions patches/sirv@2.0.2.patch
Original file line number Diff line number Diff line change
@@ -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;
21 changes: 15 additions & 6 deletions playground/assets/__tests__/assets.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,28 @@ 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')
})
})

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', () => {
Expand Down
1 change: 1 addition & 0 deletions playground/assets/static/bar
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bar
30 changes: 28 additions & 2 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit cfedf9c

Please sign in to comment.