Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: preview fallback #11312

Merged
merged 3 commits into from Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
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
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
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
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
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
@@ -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
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
@@ -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.