From 52bdda4d0fd467a14026a8b4fe5d18146f6a525b Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Mon, 1 Jul 2024 15:27:34 +0800 Subject: [PATCH 1/8] Fix attribute rendering for boolean values (#11369) --- .changeset/small-ties-sort.md | 5 +++ .../astro/src/runtime/server/render/util.ts | 21 ++++------ packages/astro/test/astro-attrs.test.js | 38 ++++++++++++++----- .../astro-attrs/src/pages/index.astro | 35 +++++++++++------ .../mdx/test/mdx-vite-env-vars.test.js | 4 +- 5 files changed, 67 insertions(+), 36 deletions(-) create mode 100644 .changeset/small-ties-sort.md 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"} /> - - - - + +

-

-

+ +

+

-

-

+ +

+

``` Astro v5.0 now preserves the full data attribute with its value when rendering the HTML of non-boolean attributes: From 351305ca23c5ea543031032827e1b4d25c53749a Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Mon, 12 Aug 2024 21:10:20 +0800 Subject: [PATCH 6/8] Typo --- .changeset/small-ties-sort.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/small-ties-sort.md b/.changeset/small-ties-sort.md index 9e73dbc101af..3c258311bb93 100644 --- a/.changeset/small-ties-sort.md +++ b/.changeset/small-ties-sort.md @@ -11,8 +11,8 @@ In the following `.astro` examples passed values of `true` and `false`, only `al ```astro -

-

+

+

From 81ab69a52cbbf083170ebcc8b10cb1aa8d73d933 Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Tue, 13 Aug 2024 18:25:28 +0800 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Armand Philippot <59021693+ArmandPhilippot@users.noreply.github.com> --- .changeset/small-ties-sort.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.changeset/small-ties-sort.md b/.changeset/small-ties-sort.md index 3c258311bb93..cce09fd71bee 100644 --- a/.changeset/small-ties-sort.md +++ b/.changeset/small-ties-sort.md @@ -26,17 +26,17 @@ In the following `.astro` examples passed values of `true` and `false`, only `al Astro v5.0 now preserves the full data attribute with its value when rendering the HTML of non-boolean attributes: ```diff -

+

-

--

-+

+

+-

++

--

-+

+-

++

-

-+

++

``` If you rely on attribute values, for example to locate elements or to conditionally render, update your code to match the new non-boolean attribute values: @@ -46,5 +46,5 @@ If you rely on attribute values, for example to locate elements or to conditiona + el.getAttribute('inherit') === 'false' - el.hasAttribute('data-light') -+ el.dataset.foo === 'true' ++ el.dataset.light === 'true' ``` From 411ceaba938daa1c0e5ed3e82949b1ef160cd99f Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Tue, 13 Aug 2024 18:25:47 +0800 Subject: [PATCH 8/8] Update .changeset/small-ties-sort.md Co-authored-by: Sarah Rainsberger --- .changeset/small-ties-sort.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/small-ties-sort.md b/.changeset/small-ties-sort.md index cce09fd71bee..e3f3d67eb4f8 100644 --- a/.changeset/small-ties-sort.md +++ b/.changeset/small-ties-sort.md @@ -6,7 +6,7 @@ Fixes attribute rendering for non-[boolean HTML attributes](https://developer.mo Previously, non-boolean attributes may not have included their values when rendered to HTML. In Astro v5.0, the values are now explicitly rendered as `="true"` or `="false"` -In the following `.astro` examples passed values of `true` and `false`, only `allowfullscreen` is a boolean attribute: +In the following `.astro` examples, only `allowfullscreen` is a boolean attribute: ```astro