diff options
| -rw-r--r-- | .changeset/cool-jokes-clap.md | 11 | ||||
| -rw-r--r-- | packages/astro/package.json | 1 | ||||
| -rw-r--r-- | packages/astro/src/runtime/server/hydration.ts | 6 | ||||
| -rw-r--r-- | packages/astro/src/runtime/server/render/component.ts | 16 | ||||
| -rw-r--r-- | packages/astro/src/runtime/server/render/util.ts | 4 | ||||
| -rw-r--r-- | packages/astro/src/runtime/server/util.ts | 30 | ||||
| -rw-r--r-- | packages/astro/test/astro-class-list.test.js | 41 | ||||
| -rw-r--r-- | packages/astro/test/fixtures/astro-class-list/src/components/Span.astro | 2 | ||||
| -rw-r--r-- | packages/astro/test/fixtures/astro-class-list/src/pages/component.astro | 8 | ||||
| -rw-r--r-- | packages/astro/test/fixtures/astro-class-list/src/pages/index.astro | 4 | ||||
| -rw-r--r-- | packages/astro/test/units/render/components.test.js | 66 | ||||
| -rw-r--r-- | pnpm-lock.yaml | 12 |
12 files changed, 133 insertions, 68 deletions
diff --git a/.changeset/cool-jokes-clap.md b/.changeset/cool-jokes-clap.md new file mode 100644 index 000000000..74176b259 --- /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 a61ca1225..770c8b9bd 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 11cce522a..81a3cf9e2 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 f78f11d51..b89bd5e56 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 f6a3f4191..e77f8ed8b 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 b38fe5ef1..97b953c72 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 9787fb458..b9d6aeba4 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 bfadf035c..a00df0b31 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 5eb64bb12..8dccbbb11 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 d8ad8aec2..13a35610d 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 0b7352453..f9dd71621 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 2e51ce7fc..45767cb01 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 |