From 823bd88f09597d2c9b01619baafdc72a2f7287ae Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Thu, 21 Sep 2023 09:21:17 +1000 Subject: [PATCH 1/5] fix: make injectQuery idempotent --- packages/vite/src/client/client.ts | 10 ++++++---- packages/vite/src/node/utils.ts | 11 +++++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/vite/src/client/client.ts b/packages/vite/src/client/client.ts index b909c1328a9a3b..965be47b15b4c6 100644 --- a/packages/vite/src/client/client.ts +++ b/packages/vite/src/client/client.ts @@ -625,11 +625,13 @@ 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 { search, searchParams, hash } = new URL(url, 'http://vitejs.dev') - return `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${ - hash || '' - }` + return searchParams.get(queryToInject) === '' + ? `${pathname}${search}${hash || ''}` + : `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${ + hash || '' + }` } export { ErrorOverlay } diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index be53f4c0636e6c..5226fcf2d295cb 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -320,12 +320,15 @@ export function injectQuery(url: string, queryToInject: string): string { url.replace(replacePercentageRE, '%25'), 'relative:///', ) - const { search, hash } = resolvedUrl + const { search, searchParams, hash } = resolvedUrl let pathname = cleanUrl(url) pathname = isWindows ? slash(pathname) : pathname - return `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${ - hash ?? '' - }` + + return searchParams.get(queryToInject) === '' + ? `${pathname}${search}${hash ?? ''}` + : `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${ + hash ?? '' + }` } const timestampRE = /\bt=\d{13}&?\b/ From 57ab5911175d910474ced8592f15492208eb62bb Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 22 Sep 2023 06:51:02 +1000 Subject: [PATCH 2/5] delete existing values for queryToInject, add tests --- packages/vite/src/client/client.ts | 14 ++++--- .../vite/src/node/__tests__/utils.spec.ts | 42 +++++++++++++++++++ packages/vite/src/node/utils.ts | 14 ++++--- 3 files changed, 58 insertions(+), 12 deletions(-) diff --git a/packages/vite/src/client/client.ts b/packages/vite/src/client/client.ts index 965be47b15b4c6..5b5249b8c466c7 100644 --- a/packages/vite/src/client/client.ts +++ b/packages/vite/src/client/client.ts @@ -625,13 +625,15 @@ 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, searchParams, hash } = new URL(url, 'http://vitejs.dev') + const { searchParams, hash } = new URL(url, 'http://vitejs.dev') - return searchParams.get(queryToInject) === '' - ? `${pathname}${search}${hash || ''}` - : `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${ - hash || '' - }` + // clean up existing query to avoid ?import&import etc. + searchParams.delete(queryToInject) + const search = searchParams.toString() + + return `${pathname}?${queryToInject}${search ? `&` + search : ''}${ + hash || '' + }` } export { ErrorOverlay } diff --git a/packages/vite/src/node/__tests__/utils.spec.ts b/packages/vite/src/node/__tests__/utils.spec.ts index 4b80e20814b82f..5b2c6bb84e65cc 100644 --- a/packages/vite/src/node/__tests__/utils.spec.ts +++ b/packages/vite/src/node/__tests__/utils.spec.ts @@ -111,6 +111,48 @@ 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, along with other query params at the start', () => { + expect( + injectQuery('/usr/vite/query?something=else&direct', 'direct'), + ).toEqual('/usr/vite/query?direct&something=else') + }) + + test('path with injected query already present, along with other query params at the end', () => { + expect( + injectQuery('/usr/vite/query?direct&something=else', 'direct'), + ).toEqual('/usr/vite/query?direct&something=else') + }) + + 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 with a value defined, along with other query params at the start', () => { + expect( + injectQuery('/usr/vite/query?something=else&direct=value', 'direct'), + ).toEqual('/usr/vite/query?direct&something=else') + }) + + test('path with injected query already present with a value defined, along with other query params at the end', () => { + expect( + injectQuery('/usr/vite/query?direct=value&something=else', 'direct'), + ).toEqual('/usr/vite/query?direct&something=else') + }) }) describe('resolveHostname', () => { diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index 5226fcf2d295cb..bacd8dc5038eda 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -320,15 +320,17 @@ export function injectQuery(url: string, queryToInject: string): string { url.replace(replacePercentageRE, '%25'), 'relative:///', ) - const { search, searchParams, hash } = resolvedUrl + const { searchParams, hash } = resolvedUrl let pathname = cleanUrl(url) pathname = isWindows ? slash(pathname) : pathname - return searchParams.get(queryToInject) === '' - ? `${pathname}${search}${hash ?? ''}` - : `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${ - hash ?? '' - }` + // clean up existing query to avoid ?import&import etc. + searchParams.delete(queryToInject) + const search = searchParams.toString() + + return `${pathname}?${queryToInject}${search ? `&` + search : ''}${ + hash ?? '' + }` } const timestampRE = /\bt=\d{13}&?\b/ From 816b82a945dd40c4c6ef319ac1142648abe17806 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 22 Sep 2023 10:07:20 +1000 Subject: [PATCH 3/5] clean up blank query values --- packages/vite/src/client/client.ts | 5 ++- .../vite/src/node/__tests__/utils.spec.ts | 40 ++++++++++++------- packages/vite/src/node/utils.ts | 5 ++- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/packages/vite/src/client/client.ts b/packages/vite/src/client/client.ts index 5b5249b8c466c7..894254bd9fdd92 100644 --- a/packages/vite/src/client/client.ts +++ b/packages/vite/src/client/client.ts @@ -629,7 +629,10 @@ export function injectQuery(url: string, queryToInject: string): string { // clean up existing query to avoid ?import&import etc. searchParams.delete(queryToInject) - const search = searchParams.toString() + const search = searchParams + .toString() + // clean up blank string values (e.g. ?vue= becomes ?vue) + .replace(/=(&|$)/g, '$1') return `${pathname}?${queryToInject}${search ? `&` + search : ''}${ hash || '' diff --git a/packages/vite/src/node/__tests__/utils.spec.ts b/packages/vite/src/node/__tests__/utils.spec.ts index 5b2c6bb84e65cc..ac29afca2eb994 100644 --- a/packages/vite/src/node/__tests__/utils.spec.ts +++ b/packages/vite/src/node/__tests__/utils.spec.ts @@ -124,34 +124,46 @@ describe('injectQuery', () => { ) }) + 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/query?something=else&direct', 'direct'), - ).toEqual('/usr/vite/query?direct&something=else') + 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/query?direct&something=else', 'direct'), - ).toEqual('/usr/vite/query?direct&something=else') - }) - - test('path with injected query already present with a value', () => { - expect(injectQuery('/usr/vite/query?direct=value', 'direct')).toEqual( - '/usr/vite/query?direct', - ) + 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/query?something=else&direct=value', 'direct'), - ).toEqual('/usr/vite/query?direct&something=else') + 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/query?direct=value&something=else', 'direct'), - ).toEqual('/usr/vite/query?direct&something=else') + injectQuery( + '/usr/vite/file.vue?direct=value&vue&type=template&lang.js', + 'direct', + ), + ).toEqual('/usr/vite/file.vue?direct&vue&type=template&lang.js') }) }) diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index bacd8dc5038eda..c41579b9217809 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -326,7 +326,10 @@ export function injectQuery(url: string, queryToInject: string): string { // clean up existing query to avoid ?import&import etc. searchParams.delete(queryToInject) - const search = searchParams.toString() + const search = searchParams + .toString() + // clean up blank string values (e.g. ?vue= becomes ?vue) + .replace(/=(&|$)/g, '$1') return `${pathname}?${queryToInject}${search ? `&` + search : ''}${ hash ?? '' From 509aedf59dd21a99fd41c4e78865fcc35aee7a9f Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Tue, 26 Sep 2023 06:10:43 +1000 Subject: [PATCH 4/5] handle key-value pairs passed to injectQuery --- packages/vite/src/client/client.ts | 2 +- packages/vite/src/node/__tests__/utils.spec.ts | 12 ++++++++++++ packages/vite/src/node/utils.ts | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/vite/src/client/client.ts b/packages/vite/src/client/client.ts index 894254bd9fdd92..4e57c9de3f765a 100644 --- a/packages/vite/src/client/client.ts +++ b/packages/vite/src/client/client.ts @@ -628,7 +628,7 @@ export function injectQuery(url: string, queryToInject: string): string { const { searchParams, hash } = new URL(url, 'http://vitejs.dev') // clean up existing query to avoid ?import&import etc. - searchParams.delete(queryToInject) + searchParams.delete(queryToInject.split('=')[0]) const search = searchParams .toString() // clean up blank string values (e.g. ?vue= becomes ?vue) diff --git a/packages/vite/src/node/__tests__/utils.spec.ts b/packages/vite/src/node/__tests__/utils.spec.ts index ac29afca2eb994..72c93e39e51b0a 100644 --- a/packages/vite/src/node/__tests__/utils.spec.ts +++ b/packages/vite/src/node/__tests__/utils.spec.ts @@ -124,12 +124,24 @@ describe('injectQuery', () => { ) }) + test('path with injected query already present when providing a key-vale pair for the injected query', () => { + expect(injectQuery('/usr/vite/query?foo', 'foo=bar')).toEqual( + '/usr/vite/query?foo=bar', + ) + }) + 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 with a value when providing a key-vale pair for the injected query', () => { + expect(injectQuery('/usr/vite/query?foo=oldValue', 'foo=newValue')).toEqual( + '/usr/vite/query?foo=newValue', + ) + }) + test('path with injected query already present, along with other query params at the start', () => { expect( injectQuery( diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index c41579b9217809..1c780cc6942b64 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -325,7 +325,7 @@ export function injectQuery(url: string, queryToInject: string): string { pathname = isWindows ? slash(pathname) : pathname // clean up existing query to avoid ?import&import etc. - searchParams.delete(queryToInject) + searchParams.delete(queryToInject.split('=')[0]) const search = searchParams .toString() // clean up blank string values (e.g. ?vue= becomes ?vue) From dd024feba4410fda92bb1d875e6d95d02f13606b Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Tue, 26 Sep 2023 06:29:00 +1000 Subject: [PATCH 5/5] fix typo --- packages/vite/src/node/__tests__/utils.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vite/src/node/__tests__/utils.spec.ts b/packages/vite/src/node/__tests__/utils.spec.ts index 72c93e39e51b0a..4357503c102c20 100644 --- a/packages/vite/src/node/__tests__/utils.spec.ts +++ b/packages/vite/src/node/__tests__/utils.spec.ts @@ -124,7 +124,7 @@ describe('injectQuery', () => { ) }) - test('path with injected query already present when providing a key-vale pair for the injected query', () => { + test('path with injected query already present when providing a key-value pair for the injected query', () => { expect(injectQuery('/usr/vite/query?foo', 'foo=bar')).toEqual( '/usr/vite/query?foo=bar', ) @@ -136,7 +136,7 @@ describe('injectQuery', () => { ) }) - test('path with injected query already present with a value when providing a key-vale pair for the injected query', () => { + test('path with injected query already present with a value when providing a key-value pair for the injected query', () => { expect(injectQuery('/usr/vite/query?foo=oldValue', 'foo=newValue')).toEqual( '/usr/vite/query?foo=newValue', )