Repository: swup/swup
Pull request #748 (2023-07-28 19:47:21)

Prevent accidental cache mutation
**Description**

Returns shallow copies instead of references for `cache.get()` and `cache.all`, to prevent unintentional cache mutation and decrease the API surface. Right now, there are two ways to mutate the cache:

```typescript

function updateCache(url: string, html: string): void {
  const page = swup.cache.get(url);
  if (!page) return;
  
  // Works, official way. Will even create the cache entry if it doesn't exist
  swup.cache.update(url, { html } as CacheData);

  // Also works, could happen unintentionally
  page.html = html;
}

```

**Drive-By**

`cache.update` won't need type casting anymore if only updating the html.

Before:

```js
swup.cache.update(url, { html } as CacheData);
swup.cache.update(url, { html, foo: "bar" } as CacheData);
```

After:

```js
swup.cache.update(url, { html });
swup.cache.update(url, { html, foo: "bar" });
```

**Checks**

<!--
Make sure the PR fulfills as many of the following requirements as possible
-->

- [x] The PR is submitted to the `master` branch
- [x] The code was linted before pushing (`npm run lint`)
- [x] All tests are passing (`npm run test`)
- [x] New or updated tests are included



Patch:
diff --git a/src/modules/Cache.ts b/src/modules/Cache.ts
index f0a1a31..f470cc6 100755
--- a/src/modules/Cache.ts
+++ b/src/modules/Cache.ts
@@ -25,7 +25,11 @@ export class Cache {
 
 	/** All cached pages. */
 	get all() {
-		return this.pages;
+		const copy = new Map();
+		this.pages.forEach((page, key) => {
+			copy.set(key, { ...page });
+		});
+		return copy;
 	}
 
 	/** Check if the given URL has been cached. */
@@ -33,9 +37,11 @@ export class Cache {
 		return this.pages.has(this.resolve(url));
 	}
 
-	/** Return the cached page object if cached. */
+	/** Return a shallow copy of the cached page object if available. */
 	get(url: string): CacheData | undefined {
-		return this.pages.get(this.resolve(url));
+		const result = this.pages.get(this.resolve(url));
+		if (!result) return result;
+		return { ...result };
 	}
 
 	/** Create a cache record for the specified URL. */
@@ -47,9 +53,9 @@ export class Cache {
 	}
 
 	/** Update a cache record, overwriting or adding custom data. */
-	update(url: string, page: CacheData) {
+	update(url: string, payload: Record<string, any>) {
 		url = this.resolve(url);
-		page = { ...this.get(url), ...page, url };
+		const page = { ...this.get(url), ...payload, url } as CacheData;
 		this.pages.set(url, page);
 	}
 


Test patch:
diff --git a/src/modules/__test__/cache.test.ts b/src/modules/__test__/cache.test.ts
index 58f3552..20fe245 100644
--- a/src/modules/__test__/cache.test.ts
+++ b/src/modules/__test__/cache.test.ts
@@ -116,6 +116,23 @@ describe('Cache', () => {
 		expect(cache.has(page2.url)).toBe(true);
 		expect(cache.has(page3.url)).toBe(false);
 	});
+
+	it('should return a copy from cache.get()', () => {
+		cache.set(page1.url, page1);
+		const page = cache.get(page1.url);
+		page!.html = 'new';
+		expect(cache.get(page1.url)?.html).toEqual(page1.html);
+	});
+
+	it('should return a new Map with shallow copies from cache.all', () => {
+		cache.set(page1.url, page1);
+		cache.set(page2.url, page2);
+
+		const all = cache.all;
+		all.get(page1.url)!.html = 'new';
+
+		expect(cache.get(page1.url)?.html).toEqual(page1.html);
+	});
 });
 
 describe('Types', () => {