Skip to content

Commit db4327b

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`. 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 db4327b

File tree

13 files changed

+142
-160
lines changed

13 files changed

+142
-160
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

+12-13
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import parse from 'url-parse'
2-
import { formatForXHR } from './url'
1+
import { formatForXHR, createfetchPath } from './url'
32
import { config } from '../config'
43
import { BasicRequestInit, ParsedResponse, RootState } from '../types'
54

@@ -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,13 @@ 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+
allData.forEach((value, key) => {
127+
fetchPath.delete(key)
128+
})
129+
130+
allData.forEach((value, key) => {
131+
fetchPath.append(key, value)
132+
})
134133
}
135134

136135
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

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

3+
const FAKE_ORIGIN = "https://example.com"
4+
5+
export function createfetchPath(path:string, baseUrl?: string) {
6+
if (baseUrl) {
7+
const parsed = new URL(url, baseUrl)
8+
return parsed.toString()
9+
} else {
10+
const parsed = new URL(url, baseUrl)
11+
return parsed.href.replace(parsed.origin, "")
12+
}
13+
}
14+
415
export function pathQuery(url: string): string {
5-
const { pathname, query } = new parse(url, {})
16+
const {pathname, search: query} = new URL(url, FAKE_ORIGIN)
617

718
return pathname + query
819
}
920

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

1324
return pathname + query + hash
1425
}
1526

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

20-
return !!query['props_at']
30+
return searchParams.has('props_at')
2131
}
2232

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

27-
return query['props_at']
36+
return searchParams.get(
37+
'props_at'
38+
)
2839
}
2940

3041
export function withFormatJson(url: string): string {
31-
const parsed = new parse(url, {}, true)
32-
parsed.query['format'] = 'json'
33-
34-
return parsed.toString()
35-
}
36-
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)
42+
const parsed = new URL(url, FAKE_ORIGIN)
43+
parsed.searchParams.set('format', 'json')
4444

45-
return pathQueryHash(parsed.toString())
45+
return parsed.href.replace(parsed.origin, "")
4646
}
4747

4848
export function removePropsAt(url: string): string {
49-
const parsed = new parse(url, {}, true)
50-
const query = parsed.query
49+
const parsed = new URL(url, FAKE_ORIGIN)
50+
parsed.searchParams.delete('props_at')
5151

52-
delete query['props_at']
53-
parsed.set('query', query)
54-
55-
return parsed.toString()
52+
return parsed.href.replace(parsed.origin, "")
5653
}
5754

5855
/**
@@ -62,29 +59,18 @@ export function removePropsAt(url: string): string {
6259
* @returns
6360
*/
6461
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)
62+
const parsed = new URL(url, FAKE_ORIGIN)
63+
parsed.searchParams.delete('props_at')
64+
parsed.searchParams.delete('format')
7165

7266
return pathQuery(parsed.toString())
7367
}
7468

7569
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)
70+
const parsed = new URL(url, FAKE_ORIGIN)
71+
parsed.hash = ""
8672

87-
return pathQuery(parsed.toString())
73+
return parsed.href.replace(parsed.origin, "")
8874
}
8975

9076
export function formatForXHR(url: string): string {
@@ -94,10 +80,15 @@ export function formatForXHR(url: string): string {
9480
}
9581

9682
export function parsePageKey(pageKey: PageKey) {
97-
const { pathname, query } = new parse(pageKey, {}, true)
83+
const {
84+
pathname,
85+
searchParams
86+
} = new URL(pageKey, FAKE_ORIGIN)
87+
88+
const search = Object.fromEntries(searchParams)
9889

9990
return {
10091
pathname,
101-
search: query,
92+
search,
10293
}
10394
}

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)