Skip to content

Commit 81ddc21

Browse files
committed
Fix dedup URL params
The depedency, url-parse, will dedup repeated url params. For example `?fop=1&foo2` and will keep the first one. This doesn't play nicely with rails Rack params as dups may indicate a structural array. This change migrates `url-parse` to the browser's own 'URLSearchParams' and `URL`. With the change, we had to: 1. Make minimal changes to `url.ts` helpers. You'll notice that we have a FAKE_ORIGIN as a fallback for the `URL` usage, but in every instances, we have no 't need the origin at all. Either we replace the origin with a blank or we only a part of the url like pathname. 2. baseUrl is required, but existing testing in isolation has it as optional. The update to the tests now reflect that requirement For dev envs, we're also included corejs which helps with adding polyfills that we can use in place for testing.
1 parent 283f0d9 commit 81ddc21

File tree

13 files changed

+129
-159
lines changed

13 files changed

+129
-159
lines changed

superglue/lib/action_creators/index.ts

+2-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { urlToPageKey, getIn } from '../utils'
2-
import parse from 'url-parse'
1+
import { urlToPageKey, getIn, propsAtParam } from '../utils'
32
import {
43
saveResponse,
54
GRAFTING_ERROR,
@@ -31,11 +30,9 @@ function fetchDeferments(
3130
successAction = GRAFTING_SUCCESS,
3231
failAction = GRAFTING_ERROR,
3332
}) {
34-
const parsedUrl = new parse(url, true)
35-
3633
// props_at will always be present in a graft response
3734
// That's why this is marked `as string`
38-
const keyPath = parsedUrl.query.props_at as string
35+
const keyPath = propsAtParam(url) as string
3936

4037
return dispatch(remote(url, { pageKey }))
4138
.then(() => {

superglue/lib/action_creators/requests.ts

-4
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import {
33
parseResponse,
44
needsRefresh,
55
urlToPageKey,
6-
withoutBusters,
76
hasPropsAt,
87
propsAtParam,
98
removePropsAt,
@@ -76,7 +75,6 @@ export const remote: RemoteCreator = (
7675
...rest
7776
} = {}
7877
) => {
79-
path = withoutBusters(path)
8078
targetPageKey = targetPageKey && urlToPageKey(targetPageKey)
8179

8280
return (dispatch, getState) => {
@@ -146,8 +144,6 @@ export const visit: VisitCreator = (
146144
...rest
147145
} = {}
148146
) => {
149-
path = withoutBusters(path)
150-
151147
return (dispatch, getState) => {
152148
const currentPageKey = getState().superglue.currentPageKey
153149
placeholderKey =

superglue/lib/components/Navigation.tsx

+1-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import React, {
66
useImperativeHandle,
77
ForwardedRef,
88
} from 'react'
9-
import { urlToPageKey, pathWithoutBZParams } from '../utils'
9+
import { urlToPageKey } from '../utils'
1010
import { removePage, setActivePage } from '../actions'
1111
import {
1212
HistoryState,
@@ -164,7 +164,6 @@ const NavigationProvider = forwardRef(function NavigationProvider(
164164
return false
165165
}
166166

167-
path = pathWithoutBZParams(path)
168167
const nextPageKey = urlToPageKey(path)
169168
const hasPage = Object.prototype.hasOwnProperty.call(
170169
store.getState().pages,

superglue/lib/index.tsx

+2-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import React, { useRef, useMemo } from 'react'
2-
import parse from 'url-parse'
32
import { config } from './config'
43
import { urlToPageKey, ujsHandlers, argsForHistory } from './utils'
54
import { saveAndProcessPage } from './action_creators'
@@ -53,8 +52,7 @@ export const prepareStore = (
5352
initialPage: VisitResponse,
5453
path: string
5554
) => {
56-
const location = parse(path)
57-
const initialPageKey = urlToPageKey(location.href)
55+
const initialPageKey = urlToPageKey(path)
5856
const { csrfToken } = initialPage
5957

6058
store.dispatch(
@@ -83,7 +81,7 @@ export const setup = ({
8381

8482
const { visit, remote } = buildVisitAndRemote(navigatorRef, store)
8583

86-
const initialPageKey = urlToPageKey(parse(path).href)
84+
const initialPageKey = urlToPageKey(path)
8785
const nextHistory = history || createHistory()
8886
nextHistory.replace(...argsForHistory(path))
8987
prepareStore(store, initialPage, path)

superglue/lib/utils/request.ts

+9-12
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import parse from 'url-parse'
21
import { formatForXHR } from './url'
32
import { config } from '../config'
43
import { BasicRequestInit, ParsedResponse, RootState } from '../types'
@@ -91,10 +90,9 @@ export function argsForFetch(
9190
nextHeaders['x-csrf-token'] = currentState.csrfToken
9291
}
9392

94-
const fetchPath = new parse(
93+
const fetchPath = new URL(
9594
formatForXHR(pathQuery),
96-
config.baseUrl || {},
97-
true
95+
config.baseUrl
9896
)
9997

10098
const credentials = 'same-origin'
@@ -113,13 +111,9 @@ export function argsForFetch(
113111
}
114112

115113
if (currentState.currentPageKey) {
116-
const referrer = new parse(
117-
currentState.currentPageKey,
118-
config.baseUrl || {},
119-
false
120-
).href
114+
const referrer = new URL(currentState.currentPageKey, config.baseUrl)
121115

122-
options.referrer = referrer
116+
options.referrer = referrer.toString()
123117
}
124118

125119
if (method == 'GET' || method == 'HEAD') {
@@ -129,8 +123,11 @@ export function argsForFetch(
129123
)
130124

131125
// TODO: Add coverage for this
132-
const nextQuery = { ...fetchPath.query, ...Object.fromEntries(allData) }
133-
fetchPath.set('query', nextQuery)
126+
// Form data should override anything in the URL params First we
127+
// delete every key. Then append the new keys accounting for
128+
// duplicate keys that represent structural arrays.
129+
allData.forEach((value, key) => fetchPath.delete(key))
130+
allData.forEach((value, key) => fetchPath.append(key, value))
134131
}
135132

136133
delete options.body

superglue/lib/utils/ujs.ts

+2-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { withoutBusters } from './url'
21
import {
32
Handlers,
43
UJSHandlers,
@@ -77,13 +76,12 @@ export class HandlerBuilder {
7776

7877
event.preventDefault()
7978

80-
let url = form.getAttribute('action')
79+
const url = form.getAttribute('action')
8180
if (!url) {
8281
return
8382
}
8483

8584
const method = (form.getAttribute('method') || 'POST').toUpperCase()
86-
url = withoutBusters(url)
8785

8886
this.visitOrRemote(form, url, {
8987
method,
@@ -103,11 +101,10 @@ export class HandlerBuilder {
103101
}
104102

105103
event.preventDefault()
106-
let url = link.getAttribute('href')
104+
const url = link.getAttribute('href')
107105
if (!url) {
108106
return
109107
}
110-
url = withoutBusters(url)
111108

112109
this.visitOrRemote(link, url, { method: 'GET' })
113110
}

superglue/lib/utils/url.ts

+30-49
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,45 @@
1-
import parse from 'url-parse'
21
import { PageKey } from '../types'
32

3+
const FAKE_ORIGIN = "https://example.com"
4+
45
export function pathQuery(url: string): string {
5-
const { pathname, query } = new parse(url, {})
6+
const {pathname, search: query} = new URL(url, FAKE_ORIGIN)
67

78
return pathname + query
89
}
910

1011
export function pathQueryHash(url: string): string {
11-
const { pathname, query, hash } = new parse(url, {})
12+
const { pathname, hash, search: query} = new URL(url, FAKE_ORIGIN)
1213

1314
return pathname + query + hash
1415
}
1516

1617
export function hasPropsAt(url: string): boolean {
17-
const parsed = new parse(url, {}, true)
18-
const query = parsed.query
18+
const { searchParams} = new URL(url, FAKE_ORIGIN)
1919

20-
return !!query['props_at']
20+
return searchParams.has('props_at')
2121
}
2222

23-
export function propsAtParam(url: string): string | undefined {
24-
const parsed = new parse(url, {}, true)
25-
const query = parsed.query
23+
export function propsAtParam(url: string): string | null {
24+
const { searchParams } = new URL(url, FAKE_ORIGIN)
2625

27-
return query['props_at']
26+
return searchParams.get(
27+
'props_at'
28+
)
2829
}
2930

3031
export function withFormatJson(url: string): string {
31-
const parsed = new parse(url, {}, true)
32-
parsed.query['format'] = 'json'
33-
34-
return parsed.toString()
35-
}
32+
const parsed = new URL(url, FAKE_ORIGIN)
33+
parsed.searchParams.set('format', 'json')
3634

37-
export function pathWithoutBZParams(url: string): string {
38-
const parsed = new parse(url, {}, true)
39-
const query = parsed.query
40-
41-
delete query['props_at']
42-
delete query['format']
43-
parsed.set('query', query)
44-
45-
return pathQueryHash(parsed.toString())
35+
return parsed.href.replace(parsed.origin, "")
4636
}
4737

4838
export function removePropsAt(url: string): string {
49-
const parsed = new parse(url, {}, true)
50-
const query = parsed.query
51-
52-
delete query['props_at']
53-
parsed.set('query', query)
39+
const parsed = new URL(url, FAKE_ORIGIN)
40+
parsed.searchParams.delete('props_at')
5441

55-
return parsed.toString()
42+
return parsed.href.replace(parsed.origin, "")
5643
}
5744

5845
/**
@@ -62,29 +49,18 @@ export function removePropsAt(url: string): string {
6249
* @returns
6350
*/
6451
export function urlToPageKey(url: string): PageKey {
65-
const parsed = new parse(url, {}, true)
66-
const query = parsed.query
67-
68-
delete query['props_at']
69-
delete query['format']
70-
parsed.set('query', query)
52+
const parsed = new URL(url, FAKE_ORIGIN)
53+
parsed.searchParams.delete('props_at')
54+
parsed.searchParams.delete('format')
7155

7256
return pathQuery(parsed.toString())
7357
}
7458

7559
export function withoutHash(url: string): string {
76-
const parsed = new parse(url, {}, true)
77-
parsed.set('hash', '')
78-
return parsed.toString()
79-
}
80-
81-
export function withoutBusters(url: string): string {
82-
const parsed = new parse(url, {}, true)
83-
const query = parsed.query
84-
delete query['format']
85-
parsed.set('query', query)
60+
const parsed = new URL(url, FAKE_ORIGIN)
61+
parsed.hash = ""
8662

87-
return pathQuery(parsed.toString())
63+
return parsed.href.replace(parsed.origin, "")
8864
}
8965

9066
export function formatForXHR(url: string): string {
@@ -94,10 +70,15 @@ export function formatForXHR(url: string): string {
9470
}
9571

9672
export function parsePageKey(pageKey: PageKey) {
97-
const { pathname, query } = new parse(pageKey, {}, true)
73+
const {
74+
pathname,
75+
searchParams
76+
} = new URL(pageKey, FAKE_ORIGIN)
77+
78+
const search = Object.fromEntries(searchParams)
9879

9980
return {
10081
pathname,
101-
search: query,
82+
search,
10283
}
10384
}

superglue/package.json

+5-6
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
"@testing-library/react": "^16.0.1",
5151
"@testing-library/user-event": "^14.5.2",
5252
"@types/node": "^22.10.1",
53-
"@types/url-parse": "^1.4.11",
5453
"@typescript-eslint/eslint-plugin": "^7.15.0",
5554
"@typescript-eslint/parser": "^7.15.0",
5655
"@vitejs/plugin-react": "^4.3.1",
@@ -76,15 +75,15 @@
7675
"typedoc-plugin-markdown": "~4.2.3",
7776
"typedoc-plugin-missing-exports": "~3.0.0",
7877
"typescript": "^5.5.3",
79-
"vitest": "^2.0.2"
78+
"vitest": "^2.0.2",
79+
"core-js": "^3.41.0"
8080
},
8181
"peerDependencies": {
82+
"@reduxjs/toolkit": "^2.2.8",
8283
"react": "^18 || ^19",
83-
"react-redux": "^9 || ^8",
84-
"@reduxjs/toolkit": "^2.2.8"
84+
"react-redux": "^9 || ^8"
8585
},
8686
"dependencies": {
87-
"history": "^5.3.0",
88-
"url-parse": "^1.5.1"
87+
"history": "^5.3.0"
8988
}
9089
}

superglue/spec/helpers/polyfill.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { AbortController } from 'abortcontroller-polyfill/dist/cjs-ponyfill'
2-
import { TextEncoder, TextDecoder } from 'util'
2+
import "core-js/stable";
33
import { JSDOM } from 'jsdom'
44

55
function setUpDomEnvironment() {
@@ -26,5 +26,3 @@ function copyProps(src, target) {
2626
setUpDomEnvironment()
2727

2828
global.AbortController = AbortController
29-
global.TextEncoder = TextEncoder
30-
global.TextDecoder = TextDecoder

superglue/spec/helpers/setup.js

+10
Original file line numberDiff line numberDiff line change
@@ -1 +1,11 @@
11
import '@testing-library/jest-dom/vitest'
2+
import {vi} from 'vitest'
3+
4+
vi.mock(import("../../lib/config.ts"), () => {
5+
return {
6+
config: {
7+
baseUrl: "https://example.com",
8+
maxPages: 20
9+
}
10+
}
11+
})

0 commit comments

Comments
 (0)