diff --git a/.changeset/small-ties-sort.md b/.changeset/small-ties-sort.md new file mode 100644 index 000000000000..2269d714d28a --- /dev/null +++ b/.changeset/small-ties-sort.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes attribute rendering for non-boolean attributes with boolean values diff --git a/packages/astro/src/runtime/server/render/util.ts b/packages/astro/src/runtime/server/render/util.ts index 469491de4edf..31a78a92eea1 100644 --- a/packages/astro/src/runtime/server/render/util.ts +++ b/packages/astro/src/runtime/server/render/util.ts @@ -8,9 +8,6 @@ export const voidElementNames = /^(area|base|br|col|command|embed|hr|img|input|keygen|link|meta|param|source|track|wbr)$/i; const htmlBooleanAttributes = /^(?:allowfullscreen|async|autofocus|autoplay|controls|default|defer|disabled|disablepictureinpicture|disableremoteplayback|formnovalidate|hidden|loop|nomodule|novalidate|open|playsinline|readonly|required|reversed|scoped|seamless|itemscope)$/i; -const htmlEnumAttributes = /^(?:contenteditable|draggable|spellcheck|value)$/i; -// Note: SVG is case-sensitive! -const svgEnumAttributes = /^(?:autoReverse|externalResourcesRequired|focusable|preserveAlpha)$/i; const AMPERSAND_REGEX = /&/g; const DOUBLE_QUOTE_REGEX = /"/g; @@ -67,13 +64,6 @@ export function addAttribute(value: any, key: string, shouldEscape = true) { return ''; } - if (value === false) { - if (htmlEnumAttributes.test(key) || svgEnumAttributes.test(key)) { - return markHTMLString(` ${key}="false"`); - } - return ''; - } - // compiler directives cannot be applied dynamically, log a warning and ignore. if (STATIC_DIRECTIVES.has(key)) { // eslint-disable-next-line no-console @@ -115,11 +105,16 @@ Make sure to use the static attribute syntax (\`${key}={value}\`) instead of the } // Boolean values only need the key - if (value === true && (key.startsWith('data-') || htmlBooleanAttributes.test(key))) { + if (htmlBooleanAttributes.test(key)) { + return markHTMLString(value ? ` ${key}` : ''); + } + + // Other attributes with an empty string value can omit rendering the value + if (value === '') { return markHTMLString(` ${key}`); - } else { - return markHTMLString(` ${key}="${toAttributeString(value, shouldEscape)}"`); } + + return markHTMLString(` ${key}="${toAttributeString(value, shouldEscape)}"`); } // Adds support for ` diff --git a/packages/astro/test/astro-attrs.test.js b/packages/astro/test/astro-attrs.test.js index f4a0140427a5..58d1ddbbb5f0 100644 --- a/packages/astro/test/astro-attrs.test.js +++ b/packages/astro/test/astro-attrs.test.js @@ -16,21 +16,41 @@ describe('Attributes', async () => { const $ = cheerio.load(html); const attrs = { - 'false-str': { attribute: 'attr', value: 'false' }, - 'true-str': { attribute: 'attr', value: 'true' }, - false: { attribute: 'attr', value: undefined }, - true: { attribute: 'attr', value: 'true' }, - empty: { attribute: 'attr', value: '' }, + 'boolean-attr-true': { attribute: 'allowfullscreen', value: '' }, + 'boolean-attr-false': { attribute: 'allowfullscreen', value: undefined }, + 'boolean-attr-string-truthy': { attribute: 'allowfullscreen', value: '' }, + 'boolean-attr-string-falsy': { attribute: 'allowfullscreen', value: undefined }, + 'boolean-attr-number-truthy': { attribute: 'allowfullscreen', value: '' }, + 'boolean-attr-number-falsy': { attribute: 'allowfullscreen', value: undefined }, + 'data-attr-true': { attribute: 'data-foobar', value: 'true' }, + 'data-attr-false': { attribute: 'data-foobar', value: 'false' }, + 'data-attr-string-truthy': { attribute: 'data-foobar', value: 'foo' }, + 'data-attr-string-falsy': { attribute: 'data-foobar', value: '' }, + 'data-attr-number-truthy': { attribute: 'data-foobar', value: '1' }, + 'data-attr-number-falsy': { attribute: 'data-foobar', value: '0' }, + 'normal-attr-true': { attribute: 'foobar', value: 'true' }, + 'normal-attr-false': { attribute: 'foobar', value: 'false' }, + 'normal-attr-string-truthy': { attribute: 'foobar', value: 'foo' }, + 'normal-attr-string-falsy': { attribute: 'foobar', value: '' }, + 'normal-attr-number-truthy': { attribute: 'foobar', value: '1' }, + 'normal-attr-number-falsy': { attribute: 'foobar', value: '0' }, null: { attribute: 'attr', value: undefined }, undefined: { attribute: 'attr', value: undefined }, - 'html-boolean': { attribute: 'async', value: 'async' }, - 'html-boolean-true': { attribute: 'async', value: 'async' }, - 'html-boolean-false': { attribute: 'async', value: undefined }, 'html-enum': { attribute: 'draggable', value: 'true' }, 'html-enum-true': { attribute: 'draggable', value: 'true' }, 'html-enum-false': { attribute: 'draggable', value: 'false' }, }; + assert.ok(!/allowfullscreen=/.test(html), 'boolean attributes should not have values'); + assert.ok( + !/id="data-attr-string-falsy"\s+data-foobar=/.test(html), + "data attributes should not have values if it's an empty string" + ); + assert.ok( + !/id="normal-attr-string-falsy"\s+data-foobar=/.test(html), + "normal attributes should not have values if it's an empty string" + ); + // cheerio will unescape the values, so checking that the url rendered unescaped to begin with has to be done manually assert.equal( html.includes('https://example.com/api/og?title=hello&description=somedescription'), @@ -46,7 +66,7 @@ describe('Attributes', async () => { for (const id of Object.keys(attrs)) { const { attribute, value } = attrs[id]; const attr = $(`#${id}`).attr(attribute); - assert.equal(attr, value); + assert.equal(attr, value, `Expected ${attribute} to be ${value} for #${id}`); } }); diff --git a/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro b/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro index 7ac96635fd14..8f2576650f62 100644 --- a/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro +++ b/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro @@ -1,19 +1,30 @@ - - - - - + + + + + + + + + + + + + + + + + + + + + + /tmp/hello.txt"} /> - - - - +