Skip to content

Commit d3fd750

Browse files
committed
fixed url to work with node18
1 parent 2459461 commit d3fd750

File tree

4 files changed

+67
-56
lines changed

4 files changed

+67
-56
lines changed

packages/svelte/src/reactivity/url-search-params.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export const ReactiveURLSearchParams = make_reactive(URLSearchParams, {
77
set: (options, ...params) => {
88
const value = options.value.get(/**@type {string} */ (params[0]));
99
const value_has_changed = value !== /**@type {string} */ (params[1]).toString();
10+
1011
if (value && !value_has_changed) {
1112
return false;
1213
}
@@ -20,6 +21,7 @@ export const ReactiveURLSearchParams = make_reactive(URLSearchParams, {
2021
}
2122

2223
options.notify_read_properties(['getAll'], params[0]);
24+
2325
return true;
2426
},
2527
append: (options, ...params) => {

packages/svelte/src/reactivity/url.js

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,26 @@ import { untrack } from '../index-client.js';
22
import { ReactiveURLSearchParams } from './url-search-params.js';
33
import { make_reactive } from './utils.js';
44

5-
//
65
// had to create a subclass for URLWithReactiveSearchParams
76
// because we cannot change the internal `searchParams` reference (which links to the web api implementation) so it requires
87
// some custom logic
9-
//
108
class URLWithReactiveSearchParams extends URL {
119
/**
1210
* @type {InstanceType<ReactiveURLSearchParams>}
1311
*/
1412
#reactive_search_params;
1513

14+
/**
15+
* @type {boolean}
16+
*/
17+
#is_in_middle_of_update = false;
18+
1619
/**
1720
* @param {ConstructorParameters<typeof URL>} params
1821
*/
1922
constructor(...params) {
2023
super(...params);
21-
22-
this.#reactive_search_params = new ReactiveURLSearchParams(super.searchParams);
24+
this.#reactive_search_params = new ReactiveURLSearchParams(super.search);
2325
}
2426

2527
/**
@@ -34,88 +36,87 @@ class URLWithReactiveSearchParams extends URL {
3436
*/
3537
get search() {
3638
this.searchParams.toString();
37-
this.#sync_params_with_url('search_params');
39+
this.#sync_params_with_url_if_not_blocked();
3840
return super.search;
3941
}
4042

4143
/**
4244
* @override
4345
*/
4446
set search(value) {
47+
this.#is_in_middle_of_update = true;
4548
super.search = value;
4649
this.#sync_params_with_url('url');
50+
this.#is_in_middle_of_update = false;
4751
}
4852

4953
/**
5054
* @override
5155
*/
5256
get href() {
5357
this.searchParams.toString();
54-
this.#sync_params_with_url('search_params');
58+
this.#sync_params_with_url_if_not_blocked();
5559
return super.href;
5660
}
5761

5862
/**
5963
* @override
6064
*/
6165
set href(value) {
66+
this.#is_in_middle_of_update = true;
6267
super.href = value;
6368
this.#sync_params_with_url('url');
69+
this.#is_in_middle_of_update = false;
6470
}
6571

6672
/**
6773
* @param {"url" | "search_params"} changed_value
6874
*/
6975
#sync_params_with_url(changed_value) {
70-
if (super.searchParams.toString() === this.searchParams.toString()) {
71-
return;
72-
}
73-
74-
if (changed_value == 'url') {
75-
this.#update_search_params_from_url();
76-
} else {
77-
// updating url from params
78-
this.search = this.searchParams.toString();
79-
}
76+
untrack(() => {
77+
if (
78+
super.search.length === 0
79+
? this.searchParams.size === 0
80+
: super.search === `?${this.searchParams}`
81+
) {
82+
return;
83+
}
84+
85+
if (changed_value == 'url') {
86+
this.#update_search_params_from_url();
87+
} else {
88+
// updating url from params
89+
this.search = this.searchParams.toString();
90+
}
91+
});
8092
}
8193

8294
#update_search_params_from_url() {
83-
/**
84-
* keeping track of this is required because we have to keep the order in which they are updated
85-
* @type {string[]}
86-
*/
87-
const keys_with_no_change = [];
88-
8995
// remove keys that don't exist anymore and notify others
9096
for (const [key, value] of Array.from(this.searchParams.entries())) {
91-
if (!super.searchParams.has(key) || value == super.searchParams.get(key)) {
92-
keys_with_no_change.push(key);
93-
untrack(() => {
94-
this.searchParams.delete(key);
95-
});
96-
continue;
97-
}
9897
this.searchParams.delete(key);
9998
}
10099

101100
// set or update keys based on the params
102101
for (const [key, value] of super.searchParams.entries()) {
103-
if (keys_with_no_change.includes(key)) {
104-
untrack(() => {
105-
this.searchParams.set(key, value);
106-
});
107-
continue;
108-
}
109102
this.searchParams.set(key, value);
110103
}
111104
}
112105

106+
#sync_params_with_url_if_not_blocked() {
107+
if (!this.#is_in_middle_of_update) {
108+
this.#is_in_middle_of_update = true;
109+
this.#sync_params_with_url('search_params');
110+
this.#is_in_middle_of_update = false;
111+
}
112+
}
113+
113114
/**
114115
* @override
115116
*/
116117
toString() {
117118
this.searchParams.toString();
118-
this.#sync_params_with_url('search_params');
119+
this.#sync_params_with_url_if_not_blocked();
119120
return super.toString();
120121
}
121122
}
@@ -201,7 +202,6 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, {
201202
options.notify_read_properties(['href', 'origin', 'host', 'port']);
202203
return true;
203204
},
204-
205205
pathname: (options, ...params) => {
206206
if (options.value.pathname === add_character_if_not_exists(params[0], '/', 'prepend')) {
207207
return false;

packages/svelte/src/reactivity/url.test.ts

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,13 @@ test('url fine grained tests', () => {
130130
};
131131
let test_description: string = '';
132132

133-
const reset_change = () => {
133+
const expect_all_changes_to_be_false = () => {
134134
for (const key of Object.keys(changes) as Array<keyof typeof url>) {
135-
changes[key] = false;
135+
assert.equal(
136+
changes[key],
137+
false,
138+
`${test_description}: effect for ${key} was not fired (value=${url[key]})`
139+
);
136140
}
137141
};
138142

@@ -142,86 +146,88 @@ test('url fine grained tests', () => {
142146
continue;
143147
}
144148
render_effect(() => {
145-
url[key];
146-
assert.equal(changes[key], true, `${test_description}: for ${key}`);
149+
const value = url[key];
150+
assert.equal(changes[key], true, `${test_description}: for ${key} (value=${value})`);
151+
changes[key] = false;
147152
});
148153
}
149154

150155
render_effect(() => {
151156
url.searchParams.get('fohoov');
152-
assert.equal(changes.searchParams, true, `${test_description}: for searchParams`);
157+
assert.equal(changes['searchParams'], true, `${test_description}: for searchParams`);
158+
changes['searchParams'] = false;
153159
});
154160
});
155161

156162
flushSync(() => {
157-
reset_change();
163+
expect_all_changes_to_be_false();
158164
changes = { ...changes, origin: true, host: true, pathname: true, href: true };
159165
test_description = 'changing href';
160166
url.href = 'https://www.google.com/test';
161167
});
162168

163169
flushSync(() => {
164-
reset_change();
170+
expect_all_changes_to_be_false();
165171
changes = { ...changes, protocol: true, origin: true, href: true };
166172
test_description = 'changing protocol';
167173
url.protocol = 'http';
168174
});
169175

170176
flushSync(() => {
171-
reset_change();
177+
expect_all_changes_to_be_false();
172178
test_description = 'changing protocol to same thing';
173179
url.protocol = 'http';
174180
});
175181

176182
flushSync(() => {
177-
reset_change();
183+
expect_all_changes_to_be_false();
178184
changes = { ...changes, hash: true, href: true };
179185
test_description = 'changing hash';
180186
url.hash = 'new-hash';
181187
});
182188

183189
flushSync(() => {
184-
reset_change();
190+
expect_all_changes_to_be_false();
185191
test_description = 'changing hash to same thing';
186192
url.hash = 'new-hash';
187193
});
188194

189195
flushSync(() => {
190-
reset_change();
196+
expect_all_changes_to_be_false();
191197
changes = { ...changes, hostname: true, host: true, href: true };
192198
test_description = 'changing hostname';
193199
url.hostname = 'fohoov';
194200
});
195201

196202
flushSync(() => {
197-
reset_change();
203+
expect_all_changes_to_be_false();
198204
changes = { ...changes, href: true, search: true, searchParams: true };
199205
test_description = 'changing search';
200206
url.search = 'fohoov=true';
201207
});
202208

203209
flushSync(() => {
204-
reset_change();
210+
expect_all_changes_to_be_false();
205211
test_description = 'changing search to same thing';
206212
url.search = 'fohoov=true';
207213
});
208214

209215
flushSync(() => {
210-
reset_change();
216+
expect_all_changes_to_be_false();
211217
changes = { ...changes, href: true, search: true, searchParams: true };
212-
test_description = 'changing search param to false';
218+
test_description = 'changing search param fohoov to false';
213219
url.search = 'fohoov=false';
214220
});
215221

216222
flushSync(() => {
217-
reset_change();
223+
expect_all_changes_to_be_false();
218224
changes = { ...changes, href: true, search: true, searchParams: true };
219225
test_description = 'clearing search';
220226
url.search = '';
221227
});
222228

223229
flushSync(() => {
224-
reset_change();
230+
expect_all_changes_to_be_false();
225231
test_description = 'clearing search again (expects no change)';
226232
url.search = '';
227233
});

packages/svelte/src/reactivity/utils.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,10 @@ function notify_read_properties(
282282
} else {
283283
(params.length == 0 ? [null] : params).forEach((param) => {
284284
const sig = get_signal_for_function(read_methods_signals, name, param);
285-
sig && increment_signal(sig);
285+
if (sig) {
286+
// I did want to use short-circuit like sig && increment_signal(sig) but linter didn't allow me to :(
287+
increment_signal(sig);
288+
}
286289
increment_version_signal(initial_version_signal_v, version_signal);
287290
});
288291
}
@@ -333,7 +336,7 @@ function get_signal_for_function(
333336
* @param {import("#client").Source<boolean>} signal
334337
*/
335338
function increment_signal(signal) {
336-
signal && set(signal, !signal.v);
339+
set(signal, !signal.v);
337340
}
338341

339342
/**

0 commit comments

Comments
 (0)