Skip to content

Commit e90f559

Browse files
bluwysarah11918ArmandPhilippot
authored
Fix attribute rendering for boolean values (take 2) (#11660)
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> Co-authored-by: Armand Philippot <59021693+ArmandPhilippot@users.noreply.github.com>
1 parent 5a3c1d1 commit e90f559

5 files changed

Lines changed: 112 additions & 36 deletions

File tree

‎.changeset/small-ties-sort.md‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
'astro': major
3+
---
4+
5+
Fixes attribute rendering for non-[boolean HTML attributes](https://developer.mozilla.org/en-US/docs/Glossary/Boolean/HTML) with boolean values to match proper attribute handling in browsers.
6+
7+
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"`
8+
9+
In the following `.astro` examples, only `allowfullscreen` is a boolean attribute:
10+
11+
```astro
12+
<!-- src/pages/index.astro -->
13+
<!-- `allowfullscreen` is a boolean attribute -->
14+
<p allowfullscreen={true}></p>
15+
<p allowfullscreen={false}></p>
16+
17+
<!-- `inherit` is *not* a boolean attribute -->
18+
<p inherit={true}></p>
19+
<p inherit={false}></p>
20+
21+
<!-- `data-*` attributes are not boolean attributes -->
22+
<p data-light={true}></p>
23+
<p data-light={false}></p>
24+
```
25+
26+
Astro v5.0 now preserves the full data attribute with its value when rendering the HTML of non-boolean attributes:
27+
28+
```diff
29+
<p allowfullscreen></p>
30+
<p></p>
31+
32+
<p inherit="true"></p>
33+
- <p inherit></p>
34+
+ <p inherit="false"></p>
35+
36+
- <p data-light></p>
37+
+ <p data-light="true"></p>
38+
- <p></p>
39+
+ <p data-light="false"></p>
40+
```
41+
42+
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:
43+
44+
```diff
45+
- el.getAttribute('inherit') === ''
46+
+ el.getAttribute('inherit') === 'false'
47+
48+
- el.hasAttribute('data-light')
49+
+ el.dataset.light === 'true'
50+
```

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

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@ export const voidElementNames =
88
/^(area|base|br|col|command|embed|hr|img|input|keygen|link|meta|param|source|track|wbr)$/i;
99
const htmlBooleanAttributes =
1010
/^(?:allowfullscreen|async|autofocus|autoplay|controls|default|defer|disabled|disablepictureinpicture|disableremoteplayback|formnovalidate|hidden|loop|nomodule|novalidate|open|playsinline|readonly|required|reversed|scoped|seamless|itemscope)$/i;
11-
const htmlEnumAttributes = /^(?:contenteditable|draggable|spellcheck|value)$/i;
12-
// Note: SVG is case-sensitive!
13-
const svgEnumAttributes = /^(?:autoReverse|externalResourcesRequired|focusable|preserveAlpha)$/i;
1411

1512
const AMPERSAND_REGEX = /&/g;
1613
const DOUBLE_QUOTE_REGEX = /"/g;
@@ -67,13 +64,6 @@ export function addAttribute(value: any, key: string, shouldEscape = true) {
6764
return '';
6865
}
6966

70-
if (value === false) {
71-
if (htmlEnumAttributes.test(key) || svgEnumAttributes.test(key)) {
72-
return markHTMLString(` ${key}="false"`);
73-
}
74-
return '';
75-
}
76-
7767
// compiler directives cannot be applied dynamically, log a warning and ignore.
7868
if (STATIC_DIRECTIVES.has(key)) {
7969
// eslint-disable-next-line no-console
@@ -115,11 +105,16 @@ Make sure to use the static attribute syntax (\`${key}={value}\`) instead of the
115105
}
116106

117107
// Boolean values only need the key
118-
if (value === true && (key.startsWith('data-') || htmlBooleanAttributes.test(key))) {
108+
if (htmlBooleanAttributes.test(key)) {
109+
return markHTMLString(value ? ` ${key}` : '');
110+
}
111+
112+
// Other attributes with an empty string value can omit rendering the value
113+
if (value === '') {
119114
return markHTMLString(` ${key}`);
120-
} else {
121-
return markHTMLString(` ${key}="${toAttributeString(value, shouldEscape)}"`);
122115
}
116+
117+
return markHTMLString(` ${key}="${toAttributeString(value, shouldEscape)}"`);
123118
}
124119

125120
// Adds support for `<Component {...value} />

‎packages/astro/test/astro-attrs.test.js‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,41 @@ describe('Attributes', async () => {
1616
const $ = cheerio.load(html);
1717

1818
const attrs = {
19-
'false-str': { attribute: 'attr', value: 'false' },
20-
'true-str': { attribute: 'attr', value: 'true' },
21-
false: { attribute: 'attr', value: undefined },
22-
true: { attribute: 'attr', value: 'true' },
23-
empty: { attribute: 'attr', value: '' },
19+
'boolean-attr-true': { attribute: 'allowfullscreen', value: '' },
20+
'boolean-attr-false': { attribute: 'allowfullscreen', value: undefined },
21+
'boolean-attr-string-truthy': { attribute: 'allowfullscreen', value: '' },
22+
'boolean-attr-string-falsy': { attribute: 'allowfullscreen', value: undefined },
23+
'boolean-attr-number-truthy': { attribute: 'allowfullscreen', value: '' },
24+
'boolean-attr-number-falsy': { attribute: 'allowfullscreen', value: undefined },
25+
'data-attr-true': { attribute: 'data-foobar', value: 'true' },
26+
'data-attr-false': { attribute: 'data-foobar', value: 'false' },
27+
'data-attr-string-truthy': { attribute: 'data-foobar', value: 'foo' },
28+
'data-attr-string-falsy': { attribute: 'data-foobar', value: '' },
29+
'data-attr-number-truthy': { attribute: 'data-foobar', value: '1' },
30+
'data-attr-number-falsy': { attribute: 'data-foobar', value: '0' },
31+
'normal-attr-true': { attribute: 'foobar', value: 'true' },
32+
'normal-attr-false': { attribute: 'foobar', value: 'false' },
33+
'normal-attr-string-truthy': { attribute: 'foobar', value: 'foo' },
34+
'normal-attr-string-falsy': { attribute: 'foobar', value: '' },
35+
'normal-attr-number-truthy': { attribute: 'foobar', value: '1' },
36+
'normal-attr-number-falsy': { attribute: 'foobar', value: '0' },
2437
null: { attribute: 'attr', value: undefined },
2538
undefined: { attribute: 'attr', value: undefined },
26-
'html-boolean': { attribute: 'async', value: 'async' },
27-
'html-boolean-true': { attribute: 'async', value: 'async' },
28-
'html-boolean-false': { attribute: 'async', value: undefined },
2939
'html-enum': { attribute: 'draggable', value: 'true' },
3040
'html-enum-true': { attribute: 'draggable', value: 'true' },
3141
'html-enum-false': { attribute: 'draggable', value: 'false' },
3242
};
3343

44+
assert.ok(!/allowfullscreen=/.test(html), 'boolean attributes should not have values');
45+
assert.ok(
46+
!/id="data-attr-string-falsy"\s+data-foobar=/.test(html),
47+
"data attributes should not have values if it's an empty string"
48+
);
49+
assert.ok(
50+
!/id="normal-attr-string-falsy"\s+data-foobar=/.test(html),
51+
"normal attributes should not have values if it's an empty string"
52+
);
53+
3454
// cheerio will unescape the values, so checking that the url rendered unescaped to begin with has to be done manually
3555
assert.equal(
3656
html.includes('https://example.com/api/og?title=hello&description=somedescription'),
@@ -46,7 +66,7 @@ describe('Attributes', async () => {
4666
for (const id of Object.keys(attrs)) {
4767
const { attribute, value } = attrs[id];
4868
const attr = $(`#${id}`).attr(attribute);
49-
assert.equal(attr, value);
69+
assert.equal(attr, value, `Expected ${attribute} to be ${value} for #${id}`);
5070
}
5171
});
5272

‎packages/astro/test/fixtures/astro-attrs/src/pages/index.astro‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,30 @@
1-
<span id="false-str" attr="false" />
2-
<span id="true-str" attr="true" />
3-
<span id="true" attr={true} />
4-
<span id="false" attr={false} />
5-
<span id="empty" attr="" />
1+
<span id="boolean-attr-true" allowfullscreen={true} />
2+
<span id="boolean-attr-false" allowfullscreen={false} />
3+
<span id="boolean-attr-string-truthy" allowfullscreen={'foo'} />
4+
<span id="boolean-attr-string-falsy" allowfullscreen={''} />
5+
<span id="boolean-attr-number-truthy" allowfullscreen={1} />
6+
<span id="boolean-attr-number-falsy" allowfullscreen={0} />
7+
8+
<span id="data-attr-true" data-foobar={true} />
9+
<span id="data-attr-false" data-foobar={false} />
10+
<span id="data-attr-string-truthy" data-foobar={'foo'} />
11+
<span id="data-attr-string-falsy" data-foobar={''} />
12+
<span id="data-attr-number-truthy" data-foobar={1} />
13+
<span id="data-attr-number-falsy" data-foobar={0} />
14+
15+
<span id="normal-attr-true" foobar={true} />
16+
<span id="normal-attr-false" foobar={false} />
17+
<span id="normal-attr-string-truthy" foobar={'foo'} />
18+
<span id="normal-attr-string-falsy" foobar={''} />
19+
<span id="normal-attr-number-truthy" foobar={1} />
20+
<span id="normal-attr-number-falsy" foobar={0} />
21+
622
<span id="null" attr={null} />
723
<span id="undefined" attr={undefined} />
24+
825
<span id="url" attr={"https://example.com/api/og?title=hello&description=somedescription"}/>
926
<span id="code" attr={"cmd: echo \"foo\" && echo \"bar\" > /tmp/hello.txt"} />
10-
<!--
11-
Per HTML spec, some attributes should be treated as booleans
12-
These should always render <span async /> or <span /> (without a string value)
13-
-->
14-
<span id='html-boolean' async />
15-
<span id='html-boolean-true' async={true} />
16-
<span id='html-boolean-false' async={false} />
27+
1728
<!--
1829
Other attributes should be treated as string enums
1930
These should always render <span draggable="true" /> or <span draggable="false" />

‎packages/integrations/mdx/test/mdx-vite-env-vars.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ describe('MDX - Vite env vars', () => {
5757
const dataAttrDump = document.querySelector('[data-env-dump]');
5858
assert.notEqual(dataAttrDump, null);
5959

60-
assert.notEqual(dataAttrDump.getAttribute('data-env-prod'), null);
61-
assert.equal(dataAttrDump.getAttribute('data-env-dev'), null);
60+
assert.equal(dataAttrDump.getAttribute('data-env-prod'), 'true');
61+
assert.equal(dataAttrDump.getAttribute('data-env-dev'), 'false');
6262
assert.equal(dataAttrDump.getAttribute('data-env-base-url'), '/');
6363
assert.equal(dataAttrDump.getAttribute('data-env-mode'), 'production');
6464
});

0 commit comments

Comments
 (0)