Replace class:list implementation with clsx (#8142)

* chore: replace `class:list` implementation with `clsx`

* chore: remove Set support from `class:list` test

* chore: better class:list test

* Update packages/astro/src/runtime/server/render/component.ts

.changeset/cool-jokes-clap.md

@@ -0,0 +1,11 @@
+---
+'astro': major
+---
+
+Fixes for the `class:list` directive
+
+- Previously, `class:list` would ocassionally not be merged the `class` prop when passed to Astro components. Now, `class:list` is always converted to a `class` prop (as a string value).
+- Previously, `class:list` diverged from [`clsx`](https://github.com/lukeed/clsx) in a few edge cases. Now, `class:list` uses [`clsx`](https://github.com/lukeed/clsx) directly.
+  - `class:list` used to deduplicate matching values, but it no longer does
+  - `class:list` used to sort individual values, but it no longer does
+  - `class:list` used to support `Set` and other iterables, but it no longer does

packages/astro/package.json

@@ -134,6 +134,7 @@
     "boxen": "^6.2.1",
     "chokidar": "^3.5.3",
     "ci-info": "^3.8.0",
+    "clsx": "^2.0.0",
     "common-ancestor-path": "^1.0.1",
     "cookie": "^0.5.0",
     "debug": "^4.3.4",

packages/astro/src/runtime/server/hydration.ts

@@ -7,7 +7,6 @@ import type {
 import { AstroError, AstroErrorData } from '../../core/errors/index.js';
 import { escapeHTML } from './escape.js';
 import { serializeProps } from './serialize.js';
-import { serializeListValue } from './util.js';
 
 export interface HydrationMetadata {
 	directive: string;
@@ -95,11 +94,6 @@ export function extractDirectives(
 					break;
 				}
 			}
-		} else if (key === 'class:list') {
-			if (value) {
-				// support "class" from an expression passed into a component (#782)
-				extracted.props[key.slice(0, -5)] = serializeListValue(value);
-			}
 		} else {
 			extracted.props[key] = value;
 		}

packages/astro/src/runtime/server/render/component.ts

@@ -6,6 +6,7 @@ import type {
 } from '../../../@types/astro';
 import type { RenderInstruction } from './types.js';
 
+import { clsx } from 'clsx';
 import { AstroError, AstroErrorData } from '../../../core/errors/index.js';
 import { HTMLBytes, markHTMLString } from '../escape.js';
 import { extractDirectives, generateHydrateScript } from '../hydration.js';
@@ -461,6 +462,9 @@ export async function renderComponent(
 		return await renderFragmentComponent(result, slots);
 	}
 
+	// Ensure directives (`class:list`) are processed
+	props = normalizeProps(props);
+
 	// .html components
 	if (isHTMLComponent(Component)) {
 		return await renderHTMLComponent(result, Component, props, slots);
@@ -473,6 +477,18 @@ export async function renderComponent(
 	return await renderFrameworkComponent(result, displayName, Component, props, slots);
 }
 
+function normalizeProps(props: Record<string, any>): Record<string, any> {
+	if (props['class:list'] !== undefined) {
+		const value = props['class:list'];
+		delete props['class:list'];
+		props['class'] = clsx(props['class'], value)
+		if (props['class'] === '') {
+			delete props['class']
+		}
+	}
+	return props;
+}
+
 export async function renderComponentToString(
 	result: SSRResult,
 	displayName: string,

packages/astro/src/runtime/server/render/util.ts

@@ -1,7 +1,7 @@
 import type { SSRElement } from '../../../@types/astro';
 
 import { HTMLString, markHTMLString } from '../escape.js';
-import { serializeListValue } from '../util.js';
+import { clsx } from 'clsx';
 
 export const voidElementNames =
 	/^(area|base|br|col|command|embed|hr|img|input|keygen|link|meta|param|source|track|wbr)$/i;
@@ -78,7 +78,7 @@ Make sure to use the static attribute syntax (\`${key}={value}\`) instead of the
 
 	// support "class" from an expression passed into an element (#782)
 	if (key === 'class:list') {
-		const listValue = toAttributeString(serializeListValue(value), shouldEscape);
+		const listValue = toAttributeString(clsx(value), shouldEscape);
 		if (listValue === '') {
 			return '';
 		}

packages/astro/src/runtime/server/util.ts

@@ -1,33 +1,3 @@
-export function serializeListValue(value: any) {
-	const hash: Record<string, any> = {};
-
-	push(value);
-
-	return Object.keys(hash).join(' ');
-
-	function push(item: any) {
-		// push individual iteratables
-		if (item && typeof item.forEach === 'function') item.forEach(push);
-		// otherwise, push object value keys by truthiness
-		else if (item === Object(item))
-			Object.keys(item).forEach((name) => {
-				if (item[name]) push(name);
-			});
-		// otherwise, push any other values as a string
-		else {
-			// get the item as a string
-			item = item === false || item == null ? '' : String(item).trim();
-
-			// add the item if it is filled
-			if (item) {
-				item.split(/\s+/).forEach((name: string) => {
-					hash[name] = true;
-				});
-			}
-		}
-	}
-}
-
 export function isPromise<T = any>(value: any): value is Promise<T> {
 	return !!value && typeof value === 'object' && typeof value.then === 'function';
 }

packages/astro/test/astro-class-list.test.js

@@ -14,14 +14,14 @@ describe('Class List', async () => {
 		const html = await fixture.readFile('/index.html');
 		const $ = cheerio.load(html);
 
-		expect($('[class="test control"]')).to.have.lengthOf(1);
-		expect($('[class="test expression"]')).to.have.lengthOf(1);
-		expect($('[class="test true"]')).to.have.lengthOf(1);
-		expect($('[class="test truthy"]')).to.have.lengthOf(1);
-		expect($('[class="test set"]')).to.have.lengthOf(1);
-		expect($('[class="hello goodbye world friend"]')).to.have.lengthOf(1);
-		expect($('[class="foo baz"]')).to.have.lengthOf(1);
-		expect($('span:not([class])')).to.have.lengthOf(1);
+		expect($('[class="test control"]')).to.have.lengthOf(1, '[class="test control"]');
+		expect($('[class="test expression"]')).to.have.lengthOf(1, '[class="test expression"]');
+		expect($('[class="test true"]')).to.have.lengthOf(1, '[class="test true"]');
+		expect($('[class="test truthy"]')).to.have.lengthOf(1, '[class="test truthy"]');
+		expect($('[class="test set"]')).to.have.lengthOf(1, '[class="test set"]');
+		expect($('[class="hello goodbye hello world hello friend"]')).to.have.lengthOf(1, '[class="hello goodbye hello world hello friend"]');
+		expect($('[class="foo baz"]')).to.have.lengthOf(1, '[class="foo baz"]');
+		expect($('span:not([class])')).to.have.lengthOf(1, 'span:not([class])');
 
 		expect($('.false, .noshow1, .noshow2, .noshow3, .noshow4')).to.have.lengthOf(0);
 	});
@@ -30,13 +30,22 @@ describe('Class List', async () => {
 		const html = await fixture.readFile('/component/index.html');
 		const $ = cheerio.load(html);
 
-		expect($('[class="test control"]')).to.have.lengthOf(1);
-		expect($('[class="test expression"]')).to.have.lengthOf(1);
-		expect($('[class="test true"]')).to.have.lengthOf(1);
-		expect($('[class="test truthy"]')).to.have.lengthOf(1);
-		expect($('[class="test set"]')).to.have.lengthOf(1);
-		expect($('[class="hello goodbye world friend"]')).to.have.lengthOf(1);
-		expect($('[class="foo baz"]')).to.have.lengthOf(1);
-		expect($('span:not([class])')).to.have.lengthOf(1);
+		expect($('[class="test control"]')).to.have.lengthOf(1, '[class="test control"]');
+		expect($('[class="test expression"]')).to.have.lengthOf(1, '[class="test expression"]');
+		expect($('[class="test true"]')).to.have.lengthOf(1, '[class="test true"]');
+		expect($('[class="test truthy"]')).to.have.lengthOf(1, '[class="test truthy"]');
+		expect($('[class="test set"]')).to.have.lengthOf(1, '[class="test set"]');
+		expect($('[class="hello goodbye hello world hello friend"]')).to.have.lengthOf(1, '[class="hello goodbye hello world hello friend"]');
+		expect($('[class="foo baz"]')).to.have.lengthOf(1, '[class="foo baz"]');
+		expect($('span:not([class])')).to.have.lengthOf(1, 'span:not([class])');
+
+		expect($('[class="test control"]').text()).to.equal('test control');
+		expect($('[class="test expression"]').text()).to.equal('test expression');
+		expect($('[class="test true"]').text()).to.equal('test true');
+		expect($('[class="test truthy"]').text()).to.equal('test truthy');
+		expect($('[class="test set"]').text()).to.equal('test set');
+		expect($('[class="hello goodbye hello world hello friend"]').text()).to.equal('hello goodbye hello world hello friend');
+		expect($('[class="foo baz"]').text()).to.equal('foo baz');
+		expect($('span:not([class])').text()).to.equal('');
 	});
 });

packages/astro/test/fixtures/astro-class-list/src/components/Span.astro

@@ -1 +1 @@
-<span {...Astro.props} />
+<span {...Astro.props}>{Astro.props.class}</span>

packages/astro/test/fixtures/astro-class-list/src/pages/component.astro

@@ -1,10 +1,8 @@
 ---
 import Component from '../components/Span.astro'
 ---
-<Component class="test control" />
 
-<!-- @note: `class:list` will not be parsed if its value is not an expression -->
-<!-- <Component class:list="test" /> -->
+<Component class="test control" />
 
 <Component class:list={'test expression'} />
 
@@ -13,9 +11,9 @@ import Component from '../components/Span.astro'
 <Component class:list={{ test: true, true: true, false: false }} />
 <Component class:list={{ test: 1, truthy: '0', noshow1: 0, noshow2: '', noshow3: null, noshow4: undefined }} />
 
-<Component class:list={new Set(['test', 'set'])} />
+<Component class:list={['test', 'set']} />
 
-<Component class:list={[ 'hello goodbye', { hello: true, world: true }, new Set([ 'hello', 'friend' ]) ]} />
+<Component class:list={[ 'hello goodbye', { hello: true, world: true }, [ 'hello', 'friend' ] ]} />
 
 <Component class:list={['foo', false && 'bar', true && 'baz']} />
 

packages/astro/test/fixtures/astro-class-list/src/pages/index.astro

@@ -10,9 +10,9 @@
 <span class:list={{ test: true, true: true, false: false }} />
 <span class:list={{ test: 1, truthy: '0', noshow1: 0, noshow2: '', noshow3: null, noshow4: undefined }} />
 
-<span class:list={new Set(['test', 'set'])} />
+<span class:list={['test', 'set']} />
 
-<span class:list={[ 'hello goodbye', { hello: true, world: true }, new Set([ 'hello', 'friend' ]) ]} />
+<span class:list={[ 'hello goodbye', { hello: true, world: true }, [ 'hello', 'friend' ] ]} />
 
 <span class:list={['foo', false && 'bar', true && 'baz']} />
 

packages/astro/test/units/render/components.test.js

@@ -1,7 +1,6 @@
 import { expect } from 'chai';
 import * as cheerio from 'cheerio';
 import { fileURLToPath } from 'node:url';
-import svelte from '../../../../integrations/svelte/dist/index.js';
 import { createFs, createRequestAndResponse, runInContainer } from '../test-utils.js';
 
 const root = new URL('../../fixtures/alias/', import.meta.url);
@@ -33,7 +32,7 @@ describe('core/render components', () => {
 				inlineConfig: {
 					root: fileURLToPath(root),
 					logLevel: 'silent',
-					integrations: [svelte()],
+					integrations: [],
 				},
 			},
 			async (container) => {
@@ -56,4 +55,67 @@ describe('core/render components', () => {
 			}
 		);
 	});
+
+	it('should merge `class` and `class:list`', async () => {
+		const fs = createFs(
+			{
+				'/src/pages/index.astro': `
+				---
+				import Class from '../components/Class.astro';
+				import ClassList from '../components/ClassList.astro';
+				import BothLiteral from '../components/BothLiteral.astro';
+				import BothFlipped from '../components/BothFlipped.astro';
+				import BothSpread from '../components/BothSpread.astro';
+				---
+				<Class class="red blue" />
+				<ClassList class:list={{ red: true, blue: true }} />
+				<BothLiteral class="red" class:list={{ blue: true }} />
+				<BothFlipped class:list={{ blue: true }} class="red" />
+				<BothSpread class:list={{ blue: true }} { ...{ class: "red" }} />
+			`,
+				'/src/components/Class.astro': `<pre id="class" set:html={JSON.stringify(Astro.props)} />`,
+				'/src/components/ClassList.astro': `<pre id="class-list" set:html={JSON.stringify(Astro.props)} />`,
+				'/src/components/BothLiteral.astro': `<pre id="both-literal" set:html={JSON.stringify(Astro.props)} />`,
+				'/src/components/BothFlipped.astro': `<pre id="both-flipped" set:html={JSON.stringify(Astro.props)} />`,
+				'/src/components/BothSpread.astro': `<pre id="both-spread" set:html={JSON.stringify(Astro.props)} />`,
+			},
+			root
+		);
+
+		await runInContainer(
+			{
+				fs,
+				inlineConfig: {
+					root: fileURLToPath(root),
+					logLevel: 'silent',
+					integrations: [],
+				},
+			},
+			async (container) => {
+				const { req, res, done, text } = createRequestAndResponse({
+					method: 'GET',
+					url: '/',
+				});
+				container.handle(req, res);
+
+				await done;
+				const html = await text();
+				const $ = cheerio.load(html);
+
+				const check = (name) => JSON.parse($(name).text() || '{}')
+
+				const Class = check('#class');
+				const ClassList = check('#class-list');
+				const BothLiteral = check('#both-literal');
+				const BothFlipped = check('#both-flipped');
+				const BothSpread = check('#both-spread');
+
+				expect(Class).to.deep.equal({ class: 'red blue' }, '#class');
+				expect(ClassList).to.deep.equal({ class: 'red blue' }, '#class-list');
+				expect(BothLiteral).to.deep.equal({ class: 'red blue' }, '#both-literal');
+				expect(BothFlipped).to.deep.equal({ class: 'red blue' }, '#both-flipped');
+				expect(BothSpread).to.deep.equal({ class: 'red blue' }, '#both-spread');
+			}
+		);
+	});
 });