fix: fix transition:name can be unicode (#9822)

* fix: fix `transition:name` can be unicode * delete prefix - * use for func * add changeset * Update .changeset/cold-bobcats-shave.md Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> * fix review issue * fix review issue * add comment * add \ to regex * fix some issue * delete unused import * remove a rule * remove valid rule -- * Update transition.ts Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> * Update transition.ts Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> * Update transition.ts Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> * Update transition.ts Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> * Update transition.ts Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> * Update transition.ts Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> * Update transition.ts Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> * format * use cssesc to escape name * test: add e2e test * add charset for layout * use the raw value for e2e test * use cssesc instead of toValidIdent * uncomment --------- Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
withastro · Jan 26, 2024 · bd880e8 · bd880e8
1 parent 00e6adb
commit bd880e8
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 6 deletions.
diff --git a/.changeset/cold-bobcats-shave.md b/.changeset/cold-bobcats-shave.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Applies the correct escaping to identifiers used with `transition:name`. 
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/Layout.astro b/packages/astro/e2e/fixtures/view-transitions/src/components/Layout.astro
@@ -22,6 +22,7 @@ const { link } = Astro.props as Props;
 		<ViewTransitions handleForms />
 		<DarkMode />
 		<meta name="script-executions" content="0">
+		<meta charset="utf-8">
 		<script is:inline defer>
 			{
 				// Increment a global to see if this is running more than once

diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/transition-name.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/transition-name.astro
@@ -0,0 +1,16 @@
+---
+import Layout from '../components/Layout.astro';
+---
+<Layout>
+	<div id="one" transition:name="front-end">front-end</div>
+	<div id="two" transition:name="开源">开源</div>
+	<div id="three" transition:name="开a源">开a源</div>
+	<div id="four" transition:name="c开a源c">c开a源c</div>
+	<div id="five" transition:name="オープンソース">オープンソース</div>
+	<div id="six" transition:name="开$源">开$源</div>
+	<div id="seven" transition:name="开.源">开.源</div>
+	<div id="eight" transition:name="🐎👱❤">🐎👱❤</div>
+	<div id="nine" transition:name="--9">--9</div>
+	<div id="ten" transition:name="10">10</div>
+	<div id="eleven" transition:name="-11">-11</div>
+</Layout>
diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js
@@ -1227,4 +1227,52 @@ test.describe('View Transitions', () => {
 
 		expect(loads.length, 'There should only be 1 page load').toEqual(1);
 	});
+
+	test('transition:name should be escaped correctly', async ({ page, astro }) => {
+		await page.goto(astro.resolveUrl('/transition-name'));
+		await expect(page.locator('#one'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'front-end'
+		);
+		await expect(page.locator('#two'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'开源'
+		);
+		await expect(page.locator('#three'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'开a源'
+		);
+		await expect(page.locator('#four'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'c开a源c'
+		);
+		await expect(page.locator('#five'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'オープンソース'
+		);
+		await expect(page.locator('#six'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'开\\$源'
+		);
+		await expect(page.locator('#seven'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'开\\.源'
+		);
+		await expect(page.locator('#eight'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'🐎👱❤'
+		);
+		await expect(page.locator('#nine'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'--9'
+		);
+		await expect(page.locator('#ten'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'\\31 0'
+		);
+		await expect(page.locator('#eleven'), 'should be escaped correctly').toHaveCSS(
+			'view-transition-name',
+			'-\\31 1'
+		);
+	});
 });
diff --git a/packages/astro/package.json b/packages/astro/package.json
@@ -134,6 +134,7 @@
     "clsx": "^2.0.0",
     "common-ancestor-path": "^1.0.1",
     "cookie": "^0.6.0",
+    "cssesc": "^3.0.0",
     "debug": "^4.3.4",
     "deterministic-object-hash": "^2.0.1",
     "devalue": "^4.3.2",
@@ -191,6 +192,7 @@
     "@types/common-ancestor-path": "^1.0.2",
     "@types/connect": "^3.4.38",
     "@types/cookie": "^0.5.4",
+    "@types/cssesc": "^3.0.2",
     "@types/debug": "^4.1.12",
     "@types/diff": "^5.0.8",
     "@types/dlv": "^1.1.4",

diff --git a/packages/astro/src/runtime/server/transition.ts b/packages/astro/src/runtime/server/transition.ts
@@ -7,6 +7,7 @@ import type {
 } from '../../@types/astro.js';
 import { fade, slide } from '../../transitions/index.js';
 import { markHTMLString } from './escape.js';
+import cssesc from 'cssesc';
 
 const transitionNameMap = new WeakMap<SSRResult, number>();
 function incrementTransitionNumber(result: SSRResult) {
@@ -23,11 +24,6 @@ export function createTransitionScope(result: SSRResult, hash: string) {
 	return `astro-${hash}-${num}`;
 }
 
-// Ensure animationName is a valid CSS identifier
-function toValidIdent(name: string): string {
-	return name.replace(/[^a-zA-Z0-9\-\_]/g, '_').replace(/^\_+|\_+$/g, '');
-}
-
 type Entries<T extends Record<string, any>> = Iterable<[keyof T, T[keyof T]]>;
 
 const getAnimations = (name: TransitionAnimationValue) => {
@@ -58,7 +54,7 @@ export function renderTransition(
 	// Default to `fade` (similar to `initial`, but snappier)
 	if (!animationName) animationName = 'fade';
 	const scope = createTransitionScope(result, hash);
-	const name = transitionName ? toValidIdent(transitionName) : scope;
+	const name = transitionName ? cssesc(transitionName, { isIdentifier: true }) : scope;
 	const sheet = new ViewTransitionStyleSheet(scope, name);
 
 	const animations = getAnimations(animationName);

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml