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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make injectQuery idempotent #14424

Closed
Closed
Show file tree
Hide file tree
Changes from 3 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
11 changes: 9 additions & 2 deletions packages/vite/src/client/client.ts
Expand Up @@ -625,9 +625,16 @@ export function injectQuery(url: string, queryToInject: string): string {

// can't use pathname from URL since it may be relative like ../
const pathname = url.replace(/#.*$/, '').replace(/\?.*$/, '')
const { search, hash } = new URL(url, 'http://vitejs.dev')
const { searchParams, hash } = new URL(url, 'http://vitejs.dev')

return `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${
// clean up existing query to avoid ?import&import etc.
searchParams.delete(queryToInject)
markdalgleish marked this conversation as resolved.
Show resolved Hide resolved
const search = searchParams
.toString()
// clean up blank string values (e.g. ?vue= becomes ?vue)
.replace(/=(&|$)/g, '$1')

return `${pathname}?${queryToInject}${search ? `&` + search : ''}${
hash || ''
}`
}
Expand Down
54 changes: 54 additions & 0 deletions packages/vite/src/node/__tests__/utils.spec.ts
Expand Up @@ -111,6 +111,60 @@ describe('injectQuery', () => {
'/usr/vite/鏉变含 %20 hello?direct',
)
})

test('path with injected query already present', () => {
expect(injectQuery('/usr/vite/query?direct', 'direct')).toEqual(
'/usr/vite/query?direct',
)
})

test('path with injected query already present multiple times', () => {
expect(injectQuery('/usr/vite/query?direct&direct', 'direct')).toEqual(
'/usr/vite/query?direct',
)
})

test('path with injected query already present with a value', () => {
expect(injectQuery('/usr/vite/query?direct=value', 'direct')).toEqual(
'/usr/vite/query?direct',
)
})

test('path with injected query already present, along with other query params at the start', () => {
expect(
injectQuery(
'/usr/vite/file.vue?vue&type=template&lang.js&direct',
'direct',
),
).toEqual('/usr/vite/file.vue?direct&vue&type=template&lang.js')
})

test('path with injected query already present, along with other query params at the end', () => {
expect(
injectQuery(
'/usr/vite/file.vue?direct&vue&type=template&lang.js',
'direct',
),
).toEqual('/usr/vite/file.vue?direct&vue&type=template&lang.js')
})

test('path with injected query already present with a value defined, along with other query params at the start', () => {
expect(
injectQuery(
'/usr/vite/file.vue?vue&type=template&lang.js&direct=value',
'direct',
),
).toEqual('/usr/vite/file.vue?direct&vue&type=template&lang.js')
})

test('path with injected query already present with a value defined, along with other query params at the end', () => {
expect(
injectQuery(
'/usr/vite/file.vue?direct=value&vue&type=template&lang.js',
'direct',
),
).toEqual('/usr/vite/file.vue?direct&vue&type=template&lang.js')
})
})

describe('resolveHostname', () => {
Expand Down
12 changes: 10 additions & 2 deletions packages/vite/src/node/utils.ts
Expand Up @@ -320,10 +320,18 @@ export function injectQuery(url: string, queryToInject: string): string {
url.replace(replacePercentageRE, '%25'),
'relative:///',
)
const { search, hash } = resolvedUrl
const { searchParams, hash } = resolvedUrl
let pathname = cleanUrl(url)
pathname = isWindows ? slash(pathname) : pathname
return `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${

// clean up existing query to avoid ?import&import etc.
searchParams.delete(queryToInject)
const search = searchParams
.toString()
// clean up blank string values (e.g. ?vue= becomes ?vue)
.replace(/=(&|$)/g, '$1')

return `${pathname}?${queryToInject}${search ? `&` + search : ''}${
hash ?? ''
}`
}
Expand Down