Skip to content

fix(curriculum): stop mutating SVG templates in addAttrs - #1946

Open
caugner wants to merge 3 commits into
mainfrom
curriculum-immutable-svg
Open

caugner wants to merge 3 commits into
mainfrom
curriculum-immutable-svg

Conversation

@caugner

@caugner caugner commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fix curriculum topic icon rendering by deriving attribute variants without mutating imported SVG templates.

Motivation

Prevent repeated SSR renders from accumulating duplicate SVG attributes and changing Lit template hashes.

Additional details

Cache derived template strings by source SVG and attribute set so Lit can reuse parsed templates.

Related issues and pull requests

Related to #936.

@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

ceb31a0 was deployed to: https://fred-pr1946.review.mdn.allizom.net/

@caugner caugner changed the title fix(curriculum): clone SVG templates before adding attributes fix(curriculum): stop mutating SVG templates in addAttrs Sep 23, 2026
@caugner
caugner force-pushed the curriculum-immutable-svg branch 3 times, most recently from fc2d4f4 to 3724046 Compare September 28, 2026 17:56
Move the helper out of `utils.js` unchanged and import it directly at
the call sites.
`addAttrs` overwrote `original.strings` on the imported SVG template,
so every SSR render appended the attributes again and gave Lit a new
strings array to parse.

Return a new result instead. Derived strings arrays are cached per
source template and attribute set, so Lit can reuse its parsed template
across renders.
@caugner
caugner force-pushed the curriculum-immutable-svg branch from 3724046 to 936fe07 Compare September 28, 2026 18:10
@caugner
caugner marked this pull request as ready for review September 28, 2026 18:14
@caugner
caugner requested a review from a team as a code owner September 28, 2026 18:14
@caugner
caugner requested a review from LeoMcA September 28, 2026 18:14

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants