Fix SVG Component sprite ids (#12511)

* fix: sprite ids per page * update tests to cover the failed scenario * add changeset
withastro · Nov 23, 2024 · d023682 · d023682
1 parent a326c2f
commit d023682
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 20 deletions.
diff --git a/.changeset/red-paws-juggle.md b/.changeset/red-paws-juggle.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fix SVG Component sprite references
diff --git a/packages/astro/src/assets/runtime.ts b/packages/astro/src/assets/runtime.ts
@@ -16,21 +16,13 @@ export interface SvgComponentProps {
 /**
  * Make sure these IDs are kept on the module-level so they're incremented on a per-page basis
  */
-const ids = new WeakMap<SSRResult, number>();
-let counter = 0;
+const countersByPage = new WeakMap<SSRResult, number>();
 
 export function createSvgComponent({ meta, attributes, children }: SvgComponentProps) {
-	const rendered = new WeakSet<Response>();
+	const renderedIds = new WeakMap<SSRResult, string>();
+
 	const Component = createComponent((result, props) => {
-		let id;
-		if (ids.has(result)) {
-			id = ids.get(result)!;
-		} else {
-			counter += 1;
-			ids.set(result, counter);
-			id = counter;
-		}
-		id = `a:${id}`;
+		let counter = countersByPage.get(result) ?? 0;
 
 		const {
 			title: titleProp,
@@ -41,12 +33,15 @@ export function createSvgComponent({ meta, attributes, children }: SvgComponentP
 		const title = titleProp ? unescapeHTML(`<title>${titleProp}</title>`) : '';
 
 		if (mode === 'sprite') {
-			// On the first render, include the symbol definition
+			// On the first render, include the symbol definition and bump the counter
 			let symbol: any = '';
-			if (!rendered.has(result.response)) {
+			let id = renderedIds.get(result);
+			if (!id) {
+				countersByPage.set(result, ++counter);
+				id = `a:${counter}`;
 				// We only need the viewBox on the symbol definition, we can drop it everywhere else
 				symbol = unescapeHTML(`<symbol${spreadAttributes({ viewBox, id })}>${children}</symbol>`);
-				rendered.add(result.response);
+				renderedIds.set(result, id);
 			}
 
 			return render`<svg${spreadAttributes(normalizedProps)}>${title}${symbol}<use href="#${id}" /></svg>`;

diff --git a/packages/astro/test/core-image-svg.test.js b/packages/astro/test/core-image-svg.test.js
@@ -322,8 +322,9 @@ describe('astro:assets - SVG Components', () => {
 				assert.equal($symbol.length, 1);
 				let $use = $('.one svg > use');
 				assert.equal($use.length, 2);
-				let defId = $('.one.def svg > use').attr('id');
-				let useId = $('.one.use svg > use').attr('id');
+				const defId = $('.one.def svg > symbol').attr('id');
+				const useId = $('.one.use svg > use').attr('href').replace('#', '');
+				assert.ok(defId);
 				assert.equal(defId, useId);
 
 				// Second SVG
@@ -333,9 +334,10 @@ describe('astro:assets - SVG Components', () => {
 				assert.equal($symbol.length, 1);
 				$use = $('.two svg > use');
 				assert.equal($use.length, 2);
-				defId = $('.two.def svg > use').attr('id');
-				useId = $('.two.use svg > use').attr('id');
-				assert.equal(defId, useId);
+				const defId2 = $('.two.def svg > symbol').attr('id');
+				const useId2 = $('.two.use svg > use').attr('href').replace('#', '');
+				assert.ok(defId2);
+				assert.equal(defId2, useId2);
 
 				// Third SVG
 				$svg = $('.three svg');
@@ -344,6 +346,12 @@ describe('astro:assets - SVG Components', () => {
 				assert.equal($symbol.length, 1);
 				$use = $('.three svg > use');
 				assert.equal($use.length, 1);
+				const defId3 = $('.three.def svg > symbol').attr('id');
+				assert.ok(defId3);
+
+				// Assert IDs are different
+				assert.equal(new Set([defId, defId2, defId3]).size, 3);
+				assert.equal(new Set([useId, useId2]).size, 2);
 			});
 		});