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

feat: base without trailing slash #10723

Merged
merged 22 commits into from Nov 12, 2022
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
0cbe2fb
chore: added tests for base without trailing slash (e.g., "/foo")
BenediktAllendorf Oct 30, 2022
62c588a
chore: remove enforcing trailing slash for base
BenediktAllendorf Oct 30, 2022
cbe04d5
chore: use joinUrlSegments() where necessary
BenediktAllendorf Oct 30, 2022
3cfa091
chore: add stripBase() for handling bases with and without trailing s…
BenediktAllendorf Oct 30, 2022
02b9ba8
Update packages/vite/src/node/plugins/importAnalysis.ts
BenediktAllendorf Nov 3, 2022
6613ab1
Update packages/vite/src/node/plugins/importAnalysis.ts
BenediktAllendorf Nov 3, 2022
3ea8891
chore: use stripBase more and fix bad calls to path.posix.join
benmccann Nov 11, 2022
0a8ea2a
chore: don't duplicate test file
benmccann Nov 12, 2022
99e0e9d
chore: merge main
benmccann Nov 12, 2022
d93cb6b
chore: leave trailing slash in resolved config per bluwy's comment
benmccann Nov 12, 2022
4d78d6a
fix: get a couple tests passing
benmccann Nov 12, 2022
3ce3ac3
fix: update package.json scripts
benmccann Nov 12, 2022
d1e9fea
fix: remove newly added broken tests
benmccann Nov 12, 2022
a4237c4
chore: cleanup package.json
benmccann Nov 12, 2022
a5efea6
feat: add rawBase field
benmccann Nov 12, 2022
3d95beb
feat: use rawBase in base middleware
benmccann Nov 12, 2022
777c81e
fix: use rawBase for tests and CLI output
benmccann Nov 12, 2022
64edcfd
fix: make test more robust
benmccann Nov 12, 2022
91f7023
chore: mark rawBase as internal
benmccann Nov 12, 2022
301ff0b
docs: update docs
benmccann Nov 12, 2022
fa03d8c
chore: merge main
benmccann Nov 12, 2022
f47f642
fix: update recently added test
benmccann Nov 12, 2022
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
2 changes: 1 addition & 1 deletion packages/vite/src/node/build.ts
Expand Up @@ -1138,7 +1138,7 @@ export function toOutputFilePathWithoutRuntime(
if (relative && !config.build.ssr) {
return toRelative(filename, hostId)
} else {
return config.base + filename
return joinUrlSegments(config.base, filename)
}
}

Expand Down
14 changes: 3 additions & 11 deletions packages/vite/src/node/config.ts
Expand Up @@ -323,6 +323,7 @@ export type ResolvedConfig = Readonly<
inlineConfig: InlineConfig
root: string
base: string
rawBase: string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's mark rawBase as internal. I would prefer to have this option as you did here, but there was a push in the last team meeting to go there only when there is a real use case out of the Vite core middleware.

publicDir: string
cacheDir: string
command: 'build' | 'serve'
Expand Down Expand Up @@ -626,7 +627,8 @@ export async function resolveConfig(
),
inlineConfig,
root: resolvedRoot,
base: resolvedBase,
base: resolvedBase.endsWith('/') ? resolvedBase : resolvedBase + '/',
rawBase: resolvedBase,
resolve: resolveOptions,
publicDir: resolvedPublicDir,
cacheDir,
Expand Down Expand Up @@ -819,12 +821,6 @@ export function resolveBaseUrl(
colors.yellow(colors.bold(`(!) "base" option should start with a slash.`))
)
}
// no ending slash warn
if (!base.endsWith('/')) {
logger.warn(
colors.yellow(colors.bold(`(!) "base" option should end with a slash.`))
)
}

// parse base when command is serve or base is not External URL
if (!isBuild || !isExternal) {
Expand All @@ -834,10 +830,6 @@ export function resolveBaseUrl(
base = '/' + base
}
}
// ensure ending slash
if (!base.endsWith('/')) {
base += '/'
}

return base
}
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/plugins/asset.ts
Expand Up @@ -247,7 +247,7 @@ function fileToDevUrl(id: string, config: ResolvedConfig) {
} else {
// outside of project root, use absolute fs path
// (this is special handled by the serve static middleware
rtn = path.posix.join(FS_PREFIX + id)
rtn = path.posix.join(FS_PREFIX, id)
}
const base = joinUrlSegments(config.server?.origin ?? '', config.base)
return joinUrlSegments(base, rtn.replace(/^\//, ''))
Expand Down
8 changes: 5 additions & 3 deletions packages/vite/src/node/plugins/css.ts
Expand Up @@ -46,6 +46,7 @@ import {
processSrcSet,
removeDirectQuery,
requireResolveFromRootWithFallback,
stripBase,
stripBomTag
} from '../utils'
import type { Logger } from '../logger'
Expand Down Expand Up @@ -265,9 +266,10 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
isCSSRequest(file)
? moduleGraph.createFileOnlyEntry(file)
: await moduleGraph.ensureEntryFromUrl(
(
await fileToUrl(file, config, this)
).replace((config.server?.origin ?? '') + devBase, '/'),
stripBase(
await fileToUrl(file, config, this),
(config.server?.origin ?? '') + devBase
),
ssr
)
)
Expand Down
14 changes: 7 additions & 7 deletions packages/vite/src/node/plugins/importAnalysis.ts
Expand Up @@ -34,10 +34,12 @@ import {
isDataUrl,
isExternalUrl,
isJSRequest,
joinUrlSegments,
moduleListContains,
normalizePath,
prettifyUrl,
removeImportQuery,
stripBase,
stripBomTag,
timeFrom,
transformStableResult,
Expand Down Expand Up @@ -263,9 +265,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
url: string,
pos: number
): Promise<[string, string]> => {
if (base !== '/' && url.startsWith(base)) {
url = url.replace(base, '/')
}
url = stripBase(url, base)

let importerFile = importer

Expand Down Expand Up @@ -319,7 +319,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
) {
// an optimized deps may not yet exists in the filesystem, or
// a regular file exists but is out of root: rewrite to absolute /@fs/ paths
url = path.posix.join(FS_PREFIX + resolved.id)
url = path.posix.join(FS_PREFIX, resolved.id)
} else {
url = resolved.id
}
Expand Down Expand Up @@ -376,8 +376,8 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
throw e
}

// prepend base (dev base is guaranteed to have ending slash)
url = base + url.replace(/^\//, '')
// prepend base
url = joinUrlSegments(base, url)
}

return [url, resolved.id]
Expand Down Expand Up @@ -538,7 +538,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {

// record for HMR import chain analysis
// make sure to unwrap and normalize away base
const hmrUrl = unwrapId(url.replace(base, '/'))
const hmrUrl = unwrapId(stripBase(url, base))
benmccann marked this conversation as resolved.
Show resolved Hide resolved
importedUrls.add(hmrUrl)

if (enablePartialAccept && importedBindings) {
Expand Down
18 changes: 9 additions & 9 deletions packages/vite/src/node/server/middlewares/base.ts
@@ -1,24 +1,23 @@
import type { Connect } from 'dep-types/connect'
import type { ViteDevServer } from '..'
import { joinUrlSegments } from '../../utils'
import { joinUrlSegments, stripBase } from '../../utils'

// this middleware is only active when (config.base !== '/')
// this middleware is only active when (base !== '/')

export function baseMiddleware({
config
}: ViteDevServer): Connect.NextHandleFunction {
const devBase = config.base.endsWith('/') ? config.base : config.base + '/'

// Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
return function viteBaseMiddleware(req, res, next) {
const url = req.url!
const parsed = new URL(url, 'http://vitejs.dev')
const path = parsed.pathname || '/'
const base = config.rawBase

if (path.startsWith(devBase)) {
if (path.startsWith(base)) {
// rewrite url to remove base. this ensures that other middleware does
// not need to consider base being prepended or not
req.url = url.replace(devBase, '/')
req.url = stripBase(url, base)
return next()
}

Expand All @@ -30,18 +29,19 @@ export function baseMiddleware({
if (path === '/' || path === '/index.html') {
// redirect root visit to based url with search and hash
res.writeHead(302, {
Location: config.base + (parsed.search || '') + (parsed.hash || '')
Location: base + (parsed.search || '') + (parsed.hash || '')
})
res.end()
return
} else if (req.headers.accept?.includes('text/html')) {
// non-based page visit
const redirectPath = joinUrlSegments(config.base, url)
const redirectPath =
url + '/' !== base ? joinUrlSegments(base, url) : base
res.writeHead(404, {
'Content-Type': 'text/html'
})
res.end(
`The server is configured with a public base URL of ${config.base} - ` +
`The server is configured with a public base URL of ${base} - ` +
`did you mean to visit <a href="${redirectPath}">${redirectPath}</a> instead?`
)
return
Expand Down
11 changes: 10 additions & 1 deletion packages/vite/src/node/utils.ts
Expand Up @@ -868,7 +868,8 @@ export async function resolveServerUrls(
const hostname = await resolveHostname(options.host)
const protocol = options.https ? 'https' : 'http'
const port = address.port
const base = config.base === './' || config.base === '' ? '/' : config.base
const base =
config.rawBase === './' || config.rawBase === '' ? '/' : config.rawBase

if (hostname.host && loopbackHosts.has(hostname.host)) {
let hostnameName = hostname.name
Expand Down Expand Up @@ -1211,6 +1212,14 @@ export function joinUrlSegments(a: string, b: string): string {
return a + b
}

export function stripBase(path: string, base: string): string {
if (path === base) {
return '/'
}
const devBase = base.endsWith('/') ? base : base + '/'
return path.replace(RegExp('^' + devBase), '/')
}

export function arrayEqual(a: any[], b: any[]): boolean {
if (a === b) return true
if (a.length !== b.length) return false
Expand Down
2 changes: 1 addition & 1 deletion playground/assets/__tests__/assets.spec.ts
Expand Up @@ -305,7 +305,7 @@ test('new URL(`${dynamic}`, import.meta.url)', async () => {

test('new URL(`non-existent`, import.meta.url)', async () => {
expect(await page.textContent('.non-existent-import-meta-url')).toMatch(
'/foo/non-existent'
new URL('non-existent', page.url()).pathname
)
})

Expand Down
2 changes: 1 addition & 1 deletion playground/assets/package.json
Expand Up @@ -3,9 +3,9 @@
"private": true,
"version": "0.0.0",
"scripts": {
"debug": "node --inspect-brk ../../packages/vite/bin/vite",
"dev": "vite",
"build": "vite build",
"debug": "node --inspect-brk ../../packages/vite/bin/vite",
"preview": "vite preview",
"dev:relative-base": "vite --config ./vite.config-relative-base.js dev",
"build:relative-base": "vite --config ./vite.config-relative-base.js build",
Expand Down
2 changes: 1 addition & 1 deletion playground/assets/vite.config.js
Expand Up @@ -4,7 +4,7 @@ const path = require('node:path')
* @type {import('vite').UserConfig}
*/
module.exports = {
base: '/foo/',
base: '/foo',
publicDir: 'static',
resolve: {
alias: {
Expand Down