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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow for fine grained invalidation of search params #11066

Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
364c2a4
feat: allow for fine grained invalidation of search params
paoloricciuti Nov 17, 2023
988bcb6
add other instances of search params functions and fix bindings
paoloricciuti Nov 17, 2023
0e51f81
fix tests for make_trackable
paoloricciuti Nov 17, 2023
47b7b9b
old and new url could be null or undefined
paoloricciuti Nov 17, 2023
ac11e75
update test
paoloricciuti Nov 17, 2023
d3b4d2f
try with logs to debug tests
paoloricciuti Nov 17, 2023
e0c67f8
remove only
paoloricciuti Nov 17, 2023
fe6b814
more logs
paoloricciuti Nov 17, 2023
4968f42
access searchParams before adding the getter and remove logs
paoloricciuti Nov 17, 2023
5373dfa
add e2e tests for fine grained invalidation
paoloricciuti Nov 17, 2023
ad8d2c6
fix linting
paoloricciuti Nov 17, 2023
bbc62f1
Add docs
paoloricciuti Nov 17, 2023
5b6b253
update docs
paoloricciuti Nov 20, 2023
d3a9ae8
add feature flag to avoid breaking change
paoloricciuti Nov 21, 2023
8fc1328
Create strange-eyes-sort.md
paoloricciuti Nov 21, 2023
d470608
fix config tests and typescript
paoloricciuti Nov 21, 2023
9e04dad
Update packages/kit/src/exports/public.d.ts
paoloricciuti Nov 23, 2023
92c45f2
Update packages/kit/src/utils/url.spec.js
paoloricciuti Nov 23, 2023
e8bb9f1
Update packages/kit/src/runtime/server/respond.js
paoloricciuti Nov 23, 2023
f4975dd
Update documentation/docs/20-core-concepts/20-load.md
paoloricciuti Nov 23, 2023
6172001
Update documentation/docs/20-core-concepts/20-load.md
paoloricciuti Nov 23, 2023
51c20ab
remove todos and function check
paoloricciuti Nov 23, 2023
23ea3f5
update check_search_params_changed logic to handle multiple search pa…
paoloricciuti Nov 23, 2023
56b50d5
specify where to set fineGrainedSearchParamsInvalidation
paoloricciuti Nov 30, 2023
d72cac9
use vite define to access fine grained config
paoloricciuti Nov 30, 2023
ff8b4bb
remove config since will be on the 2.0 milestone
paoloricciuti Dec 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions documentation/docs/20-core-concepts/20-load.md
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,8 @@ A `load` function that calls `await parent()` will also rerun if a parent `load`

Dependency tracking does not apply _after_ the `load` function has returned — for example, accessing `params.x` inside a nested [promise](#streaming-with-promises) will not cause the function to rerun when `params.x` changes. (Don't worry, you'll get a warning in development if you accidentally do this.) Instead, access the parameter in the main body of your `load` function.

Also accessing one single query parameter is tracked independently from the rest of the url. This means that accessing `event.url.searchParams.get("query")` inside a `load` function will make that load function rerun only when the `query` search param changes. For example navigating from `/search?query=svelte&page=1` to `/search?query=svelte&page=2` will not rerun a load function that access `event.url.searchParams.get("query")` but not `event.url.searchParams.get("page")`.
paoloricciuti marked this conversation as resolved.
Show resolved Hide resolved
paoloricciuti marked this conversation as resolved.
Show resolved Hide resolved

### Manual invalidation

You can also rerun `load` functions that apply to the current page using [`invalidate(url)`](modules#$app-navigation-invalidate), which reruns all `load` functions that depend on `url`, and [`invalidateAll()`](modules#$app-navigation-invalidateall), which reruns every `load` function. Server load functions will never automatically depend on a fetched `url` to avoid leaking secrets to the client.
Expand Down
72 changes: 63 additions & 9 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,8 @@ export function create_client(app, target) {
params: new Set(),
parent: false,
route: false,
url: false
url: false,
search_params: new Set()
};

const node = await loader();
Expand Down Expand Up @@ -478,9 +479,15 @@ export function create_client(app, target) {
}
}),
data: server_data_node?.data ?? null,
url: make_trackable(url, () => {
uses.url = true;
}),
url: make_trackable(
url,
() => {
uses.url = true;
},
(search_param) => {
uses.search_params.add(search_param);
}
),
async fetch(resource, init) {
/** @type {URL | string} */
let requested;
Expand Down Expand Up @@ -576,10 +583,18 @@ export function create_client(app, target) {
* @param {boolean} parent_changed
* @param {boolean} route_changed
* @param {boolean} url_changed
* @param {Set<string>} search_params_changed
* @param {import('types').Uses | undefined} uses
* @param {Record<string, string>} params
*/
function has_changed(parent_changed, route_changed, url_changed, uses, params) {
function has_changed(
parent_changed,
route_changed,
url_changed,
search_params_changed,
uses,
params
) {
if (force_invalidation) return true;

if (!uses) return false;
Expand All @@ -588,6 +603,10 @@ export function create_client(app, target) {
if (uses.route && route_changed) return true;
if (uses.url && url_changed) return true;

for (const tracked_params of uses.search_params) {
if (search_params_changed.has(tracked_params)) return true;
}

for (const param of uses.params) {
if (params[param] !== current.params[param]) return true;
}
Expand All @@ -610,6 +629,26 @@ export function create_client(app, target) {
return null;
}

/**
*
* @param {URL} [old_url]
* @param {URL} [new_url]
*/
function check_search_params_changed(old_url, new_url) {
const changed = new Set();
const new_search_params = new URLSearchParams(new_url?.searchParams);
for (const [key, value] of old_url?.searchParams?.entries?.() ?? []) {
if (new_search_params.get(key) !== value) {
changed.add(key);
}
new_search_params.delete(key);
paoloricciuti marked this conversation as resolved.
Show resolved Hide resolved
}
for (const [key] of new_search_params) {
changed.add(key);
}
return changed;
}

/**
* @param {import('./types.js').NavigationIntent} intent
* @returns {Promise<import('./types.js').NavigationResult>}
Expand All @@ -631,9 +670,9 @@ export function create_client(app, target) {

/** @type {import('types').ServerNodesResponse | import('types').ServerRedirectNode | null} */
let server_data = null;

const url_changed = current.url ? id !== current.url.pathname + current.url.search : false;
const route_changed = current.route ? route.id !== current.route.id : false;
const search_params_changed = check_search_params_changed(current.url, url);

let parent_invalid = false;
const invalid_server_nodes = loaders.map((loader, i) => {
Expand All @@ -642,7 +681,14 @@ export function create_client(app, target) {
const invalid =
!!loader?.[0] &&
(previous?.loader !== loader[1] ||
has_changed(parent_invalid, route_changed, url_changed, previous.server?.uses, params));
has_changed(
parent_invalid,
route_changed,
url_changed,
search_params_changed,
previous.server?.uses,
params
));

if (invalid) {
// For the next one
Expand Down Expand Up @@ -685,7 +731,14 @@ export function create_client(app, target) {
const valid =
(!server_data_node || server_data_node.type === 'skip') &&
loader[1] === previous?.loader &&
!has_changed(parent_changed, route_changed, url_changed, previous.universal?.uses, params);
!has_changed(
parent_changed,
route_changed,
url_changed,
search_params_changed,
previous.universal?.uses,
params
);
if (valid) return previous;

parent_changed = true;
Expand Down Expand Up @@ -1954,7 +2007,8 @@ function deserialize_uses(uses) {
params: new Set(uses?.params ?? []),
parent: !!uses?.parent,
route: !!uses?.route,
url: !!uses?.url
url: !!uses?.url,
search_params: new Set(uses?.search_params ?? [])
};
}

Expand Down
30 changes: 21 additions & 9 deletions packages/kit/src/runtime/server/page/load_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,30 @@ export async function load_server_data({
params: new Set(),
parent: false,
route: false,
url: false
url: false,
search_params: new Set()
};

const url = make_trackable(event.url, () => {
if (DEV && done && !uses.url) {
console.warn(
`${node.server_id}: Accessing URL properties in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the URL changes`
);
}
const url = make_trackable(
event.url,
() => {
if (DEV && done && !uses.url) {
console.warn(
`${node.server_id}: Accessing URL properties in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the URL changes`
);
}

uses.url = true;
});
uses.url = true;
},
(search_params) => {
if (DEV && done && uses.search_params.size === 0) {
console.warn(
`${node.server_id}: Accessing URL properties in a promise handler after \`load(...)\` has returned will not cause the function to re-run when the URL changes`
);
}
uses.search_params.add(search_params);
}
);

if (state.prerendering) {
disable_search(url);
Expand Down
4 changes: 4 additions & 0 deletions packages/kit/src/runtime/server/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ export function stringify_uses(node) {
uses.push(`"dependencies":${JSON.stringify(Array.from(node.uses.dependencies))}`);
}

if (node.uses && node.uses.search_params.size > 0) {
uses.push(`"search_params":${JSON.stringify(Array.from(node.uses.search_params))}`);
}

if (node.uses && node.uses.params.size > 0) {
uses.push(`"params":${JSON.stringify(Array.from(node.uses.params))}`);
}
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ export interface Uses {
parent: boolean;
route: boolean;
url: boolean;
search_params: Set<string>;
}

export type ValidatedConfig = RecursiveRequired<Config>;
Expand Down
45 changes: 43 additions & 2 deletions packages/kit/src/utils/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,50 @@ const tracked_url_properties = /** @type {const} */ ([
'href',
'pathname',
'search',
'searchParams',
'toString',
'toJSON'
]);

const tracked_search_params_properties = /** @type {const} */ ['get', 'getAll', 'has'];

/**
* @param {URLSearchParams} search_params
* @param {() => void} callback
* @param {(search_param: string) => void} search_params_callback
*/
function tracked_search_params(search_params, callback, search_params_callback) {
return new Proxy(search_params, {
get(obj, key) {
if (typeof key === 'string' && tracked_search_params_properties.includes(key)) {
// @ts-expect-error
const function_to_call = key in search_params ? search_params[key].bind(search_params) : '';
if (function_to_call && function_to_call instanceof Function) {
paoloricciuti marked this conversation as resolved.
Show resolved Hide resolved
return (/**@type {string}*/ search_param) => {
search_params_callback(search_param);
return function_to_call(search_param);
};
}
}
// if they try to access something different from what is in `tracked_search_params_properties`
// we track the whole url (entries, values, keys etc)
callback();
let retval = Reflect.get(obj, key);
if (retval instanceof Function) {
retval = retval.bind(search_params);
}
return retval;
}
});
}

/**
* @param {URL} url
* @param {() => void} callback
* @param {(search_param: string) => void} search_params_callback
*/
export function make_trackable(url, callback) {
export function make_trackable(url, callback, search_params_callback) {
const tracked = new URL(url);
const search_params_to_track = new URLSearchParams(tracked.searchParams);

for (const property of tracked_url_properties) {
Object.defineProperty(tracked, property, {
Expand All @@ -127,6 +160,14 @@ export function make_trackable(url, callback) {
});
}

Object.defineProperty(tracked, 'searchParams', {
get() {
return tracked_search_params(search_params_to_track, callback, search_params_callback);
},
enumerable: true,
configurable: true
});

if (!BROWSER) {
// @ts-ignore
tracked[Symbol.for('nodejs.util.inspect.custom')] = (depth, opts, inspect) => {
Expand Down
45 changes: 40 additions & 5 deletions packages/kit/src/utils/url.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,13 @@ describe('normalize_path', (test) => {
describe('make_trackable', (test) => {
test('makes URL properties trackable', () => {
let tracked = false;

const url = make_trackable(new URL('https://kit.svelte.dev/docs'), () => {
tracked = true;
});
const url = make_trackable(
new URL('https://kit.svelte.dev/docs'),
() => {
tracked = true;
},
() => {}
);

url.origin;
assert.isNotOk(tracked);
Expand All @@ -106,13 +109,45 @@ describe('make_trackable', (test) => {
});

test('throws an error when its hash property is accessed', () => {
const url = make_trackable(new URL('https://kit.svelte.dev/docs'), () => {});
const url = make_trackable(
new URL('https://kit.svelte.dev/docs'),
() => {},
() => {}
);

assert.throws(
() => url.hash,
/Cannot access event.url.hash. Consider using `\$page.url.hash` inside a component instead/
);
});
test('track each search param separately if accessed directly', () => {
paoloricciuti marked this conversation as resolved.
Show resolved Hide resolved
let tracked = false;
const tracked_search_params = new Set();
const url = make_trackable(
new URL('https://kit.svelte.dev/docs'),
() => {
tracked = true;
},
(search_param) => {
tracked_search_params.add(search_param);
}
);

url.searchParams.get('test');
assert.isNotOk(tracked);
assert.ok(tracked_search_params.has('test'));

url.searchParams.getAll('test-getall');
assert.isNotOk(tracked);
assert.ok(tracked_search_params.has('test-getall'));

url.searchParams.has('test-has');
assert.isNotOk(tracked);
assert.ok(tracked_search_params.has('test-has'));

url.searchParams.entries();
assert.ok(tracked);
});
});

describe('disable_search', (test) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
let count = 0;

export function load({ url: { searchParams } }) {
searchParams.get('test');
return {
count: count++
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
export let data;
</script>

<span>count: {data.count}</span>
<a href={`?test=${data.count + 1}`}>Change test</a>
<a href={`?test=${data.count}&another=another`}>Change another param</a>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
let count = 0;

export function load({ url: { searchParams } }) {
searchParams.get('test');
return {
count: count++
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
export let data;
</script>

<span>count: {data.count}</span>
<a href={`?test=${data.count + 1}`}>Change test</a>
<a href={`?test=${data.count}&another=another`}>Change another param</a>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
let count = 0;

export function load({ url: { searchParams } }) {
searchParams.get('test');
return {
count: count++
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
export let data;
</script>

<span>count: {data.count}</span>
<a href={`?test=${data.count + 1}`}>Change test</a>
<a href={`?test=${data.count}&another=another`}>Change another param</a>
Loading
Loading