fix: correctly detect changed data (#8377)

fixes #8157

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>

.changeset/grumpy-adults-breathe.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: correctly detect changed data

packages/kit/src/runtime/client/client.js

@@ -32,6 +32,7 @@ import { unwrap_promises } from '../../utils/promises.js';
 import * as devalue from 'devalue';
 import { INDEX_KEY, PRELOAD_PRIORITIES, SCROLL_KEY } from './constants.js';
 import { validate_common_exports } from '../../utils/exports.js';
+import { compact } from '../../utils/array.js';
 
 const routes = parse(nodes, server_loads, dictionary, matchers);
 
@@ -414,8 +415,6 @@ export function create_client({ target, base }) {
 		route,
 		form
 	}) {
-		const filtered = /** @type {import('./types').BranchNode[] } */ (branch.filter(Boolean));
-
 		/** @type {import('types').TrailingSlash} */
 		let slash = 'never';
 		for (const node of branch) {
@@ -436,7 +435,7 @@ export function create_client({ target, base }) {
 			},
 			props: {
 				// @ts-ignore Somehow it's getting SvelteComponent and SvelteComponentDev mixed up
-				components: filtered.map((branch_node) => branch_node.node.component)
+				components: compact(branch).map((branch_node) => branch_node.node.component)
 			}
 		};
 
@@ -446,21 +445,24 @@ export function create_client({ target, base }) {
 
 		let data = {};
 		let data_changed = !page;
-		for (let i = 0; i < filtered.length; i += 1) {
-			const node = filtered[i];
+
+		let p = 0;
+
+		for (let i = 0; i < Math.max(branch.length, current.branch.length); i += 1) {
+			const node = branch[i];
+			const prev = current.branch[i];
+
+			if (node?.data !== prev?.data) data_changed = true;
+			if (!node) continue;
+
 			data = { ...data, ...node.data };
 
 			// Only set props if the node actually updated. This prevents needless rerenders.
-			if (data_changed || !current.branch.some((previous) => previous === node)) {
-				result.props[`data_${i}`] = data;
-				data_changed = data_changed || Object.keys(node.data ?? {}).length > 0;
+			if (data_changed) {
+				result.props[`data_${p}`] = data;
 			}
-		}
-		if (!data_changed) {
-			// If nothing was added, and the object entries are the same length, this means
-			// that nothing was removed either and therefore the data is the same as the previous one.
-			// This would be more readable with a separate boolean but that would cost us some bytes.
-			data_changed = Object.keys(page.data).length !== Object.keys(data).length;
+
+			p += 1;
 		}
 
 		const page_changed =

packages/kit/test/apps/basics/src/routes/load/unchanged/+page.svelte

@@ -1,2 +1 @@
-<a href="/load/unchanged/uses-parent/a">uses parent</a>
 <a href="/load/unchanged/isolated/a">isolated</a>

packages/kit/test/apps/basics/src/routes/store/data/unchanged/+layout.svelte → packages/kit/test/apps/basics/src/routes/store/data/store-update/+layout.svelte

@@ -10,5 +10,6 @@
 </script>
 
 <p>$page.data was updated {count} time(s)</p>
-<a href="/store/data/unchanged/a">a</a>
-<a href="/store/data/unchanged/b">b</a>
+<a href="/store/data/store-updates/a">a</a>
+<a href="/store/data/store-updates/b">b</a>
+<slot />

packages/kit/test/apps/basics/src/routes/store/data/store-update/same-keys/+layout.js

@@ -0,0 +1,5 @@
+export function load() {
+	return {
+		value: 'layout'
+	};
+}

packages/kit/test/apps/basics/src/routes/store/data/store-update/same-keys/same-deep/nested/+page.js

@@ -0,0 +1,5 @@
+export function load() {
+	return {
+		value: 'page'
+	};
+}

packages/kit/test/apps/basics/src/routes/store/data/store-update/same-keys/same/+page.js

@@ -0,0 +1,5 @@
+export function load() {
+	return {
+		value: 'page'
+	};
+}

packages/kit/test/apps/basics/test/client.test.js

@@ -918,10 +918,22 @@ test.describe('$app/stores', () => {
 	});
 
 	test('$page.data does not update if data is unchanged', async ({ page, app }) => {
-		await page.goto('/store/data/unchanged/a');
-		await app.goto('/store/data/unchanged/b');
+		await page.goto('/store/data/store-update/a');
+		await app.goto('/store/data/store-update/b');
 		await expect(page.locator('p')).toHaveText('$page.data was updated 0 time(s)');
 	});
+
+	test('$page.data does update if keys did not change but data did', async ({ page, app }) => {
+		await page.goto('/store/data/store-update/same-keys/same');
+		await app.goto('/store/data/store-update/same-keys');
+		await expect(page.locator('p')).toHaveText('$page.data was updated 1 time(s)');
+	});
+
+	test('$page.data does update if keys did not change but data did (2)', async ({ page, app }) => {
+		await page.goto('/store/data/store-update/same-keys/same-deep/nested');
+		await app.goto('/store/data/store-update/same-keys');
+		await expect(page.locator('p')).toHaveText('$page.data was updated 1 time(s)');
+	});
 });
 
 test.describe('Invalidation', () => {