From 81545197a32fd015d763fc386c8b67e0e08b7393 Mon Sep 17 00:00:00 2001
From: Nate Moore <natemoo-re@users.noreply.github.com>
Date: Fri, 18 Aug 2023 15:25:57 -0500
Subject: [PATCH] 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                 | 11 ++++
 packages/astro/package.json                   |  1 +
 .../astro/src/runtime/server/hydration.ts     |  6 --
 .../src/runtime/server/render/component.ts    | 16 +++++
 .../astro/src/runtime/server/render/util.ts   |  4 +-
 packages/astro/src/runtime/server/util.ts     | 30 ---------
 packages/astro/test/astro-class-list.test.js  | 41 +++++++-----
 .../src/components/Span.astro                 |  2 +-
 .../src/pages/component.astro                 |  8 +--
 .../astro-class-list/src/pages/index.astro    |  4 +-
 .../test/units/render/components.test.js      | 66 ++++++++++++++++++-
 pnpm-lock.yaml                                | 12 ++--
 12 files changed, 133 insertions(+), 68 deletions(-)
 create mode 100644 .changeset/cool-jokes-clap.md

diff --git a/.changeset/cool-jokes-clap.md b/.changeset/cool-jokes-clap.md
new file mode 100644
index 000000000000..74176b259447
--- /dev/null
+++ b/.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
diff --git a/packages/astro/package.json b/packages/astro/package.json
index a61ca1225f92..770c8b9bdfcc 100644
--- a/packages/astro/package.json
+++ b/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",
diff --git a/packages/astro/src/runtime/server/hydration.ts b/packages/astro/src/runtime/server/hydration.ts
index 11cce522aa3a..81a3cf9e200e 100644
--- a/packages/astro/src/runtime/server/hydration.ts
+++ b/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;
 		}
diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts
index f78f11d517f7..b89bd5e56a56 100644
--- a/packages/astro/src/runtime/server/render/component.ts
+++ b/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,
diff --git a/packages/astro/src/runtime/server/render/util.ts b/packages/astro/src/runtime/server/render/util.ts
index f6a3f4191009..e77f8ed8b06d 100644
--- a/packages/astro/src/runtime/server/render/util.ts
+++ b/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 '';
 		}
diff --git a/packages/astro/src/runtime/server/util.ts b/packages/astro/src/runtime/server/util.ts
index b38fe5ef1b9f..97b953c726f4 100644
--- a/packages/astro/src/runtime/server/util.ts
+++ b/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';
 }
diff --git a/packages/astro/test/astro-class-list.test.js b/packages/astro/test/astro-class-list.test.js
index 9787fb458e21..b9d6aeba459b 100644
--- a/packages/astro/test/astro-class-list.test.js
+++ b/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('');
 	});
 });
diff --git a/packages/astro/test/fixtures/astro-class-list/src/components/Span.astro b/packages/astro/test/fixtures/astro-class-list/src/components/Span.astro
index bfadf035c518..a00df0b31866 100644
--- a/packages/astro/test/fixtures/astro-class-list/src/components/Span.astro
+++ b/packages/astro/test/fixtures/astro-class-list/src/components/Span.astro
@@ -1 +1 @@
-<span {...Astro.props} />
+<span {...Astro.props}>{Astro.props.class}</span>
diff --git a/packages/astro/test/fixtures/astro-class-list/src/pages/component.astro b/packages/astro/test/fixtures/astro-class-list/src/pages/component.astro
index 5eb64bb12a87..8dccbbb11b48 100644
--- a/packages/astro/test/fixtures/astro-class-list/src/pages/component.astro
+++ b/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']} />
 
diff --git a/packages/astro/test/fixtures/astro-class-list/src/pages/index.astro b/packages/astro/test/fixtures/astro-class-list/src/pages/index.astro
index d8ad8aec2a20..13a35610d372 100644
--- a/packages/astro/test/fixtures/astro-class-list/src/pages/index.astro
+++ b/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']} />
 
diff --git a/packages/astro/test/units/render/components.test.js b/packages/astro/test/units/render/components.test.js
index 0b73524537fe..f9dd71621fa0 100644
--- a/packages/astro/test/units/render/components.test.js
+++ b/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');
+			}
+		);
+	});
 });
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index 2e51ce7fc750..45767cb015cd 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -527,6 +527,9 @@ importers:
       ci-info:
         specifier: ^3.8.0
         version: 3.8.0
+      clsx:
+        specifier: ^2.0.0
+        version: 2.0.0
       common-ancestor-path:
         specifier: ^1.0.1
         version: 1.0.1
@@ -10150,6 +10153,11 @@ packages:
     resolution: {integrity: sha512-JQHZ2QMW6l3aH/j6xCqQThY/9OH4D/9ls34cgkUBiEeocRTU04tHfKPBsUK1PqZCUQM7GiA0IIXJSuXHI64Kbg==}
     engines: {node: '>=0.8'}
 
+  /clsx@2.0.0:
+    resolution: {integrity: sha512-rQ1+kcj+ttHG0MKVGBUXwayCCF1oh39BF5COIpRzuCEv8Mwjv0XucrI2ExNTOn9IlLifGClWQcU9BrZORvtw6Q==}
+    engines: {node: '>=6'}
+    dev: false
+
   /code-block-writer@12.0.0:
     resolution: {integrity: sha512-q4dMFMlXtKR3XNBHyMHt/3pwYNA69EDk00lloMOaaUMKPUXBw6lpXtbu3MMVG6/uOihGnRDOlkyqsONEUj60+w==}
     dev: true
@@ -18158,25 +18166,21 @@ packages:
   file:packages/astro/test/fixtures/css-assets/packages/font-awesome:
     resolution: {directory: packages/astro/test/fixtures/css-assets/packages/font-awesome, type: directory}
     name: '@test/astro-font-awesome-package'
-    version: 0.0.1
     dev: false
 
   file:packages/astro/test/fixtures/multiple-renderers/renderers/one:
     resolution: {directory: packages/astro/test/fixtures/multiple-renderers/renderers/one, type: directory}
     name: '@test/astro-renderer-one'
-    version: 1.0.0
     dev: false
 
   file:packages/astro/test/fixtures/multiple-renderers/renderers/two:
     resolution: {directory: packages/astro/test/fixtures/multiple-renderers/renderers/two, type: directory}
     name: '@test/astro-renderer-two'
-    version: 1.0.0
     dev: false
 
   file:packages/astro/test/fixtures/solid-component/deps/solid-jsx-component:
     resolution: {directory: packages/astro/test/fixtures/solid-component/deps/solid-jsx-component, type: directory}
     name: '@test/solid-jsx-component'
-    version: 0.0.0
     dependencies:
       solid-js: 1.7.6
     dev: false