Revert "Fix attribute rendering for boolean values (#11369)" (#11408)

* Revert "Fix attribute rendering for boolean values (#11369)"

This reverts commit e6de11f.

* Add a changeset

.changeset/calm-rules-trade.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Revert change to how boolean attributes work

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

@@ -8,6 +8,9 @@ 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;
@@ -64,6 +67,13 @@ 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
@@ -105,16 +115,11 @@ Make sure to use the static attribute syntax (\`${key}={value}\`) instead of the
 	}
 
 	// Boolean values only need the key
-	if (htmlBooleanAttributes.test(key)) {
-		return markHTMLString(value ? ` ${key}` : '');
-	}
-
-	// Other attributes with an empty string value can omit rendering the value
-	if (value === '') {
+	if (value === true && (key.startsWith('data-') || htmlBooleanAttributes.test(key))) {
 		return markHTMLString(` ${key}`);
+	} else {
+		return markHTMLString(` ${key}="${toAttributeString(value, shouldEscape)}"`);
 	}
-
-	return markHTMLString(` ${key}="${toAttributeString(value, shouldEscape)}"`);
 }
 
 // Adds support for `<Component {...value} />

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

@@ -16,41 +16,21 @@ describe('Attributes', async () => {
 		const $ = cheerio.load(html);
 
 		const attrs = {
-			'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' },
+			'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: '' },
 			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'),
@@ -66,7 +46,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, `Expected ${attribute} to be ${value} for #${id}`);
+			assert.equal(attr, value);
 		}
 	});
 

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

@@ -1,30 +1,19 @@
-<span id="boolean-attr-true" allowfullscreen={true} />
-<span id="boolean-attr-false" allowfullscreen={false} />
-<span id="boolean-attr-string-truthy" allowfullscreen={'foo'} />
-<span id="boolean-attr-string-falsy" allowfullscreen={''} />
-<span id="boolean-attr-number-truthy" allowfullscreen={1} />
-<span id="boolean-attr-number-falsy" allowfullscreen={0} />
-
-<span id="data-attr-true" data-foobar={true} />
-<span id="data-attr-false" data-foobar={false} />
-<span id="data-attr-string-truthy" data-foobar={'foo'} />
-<span id="data-attr-string-falsy" data-foobar={''} />
-<span id="data-attr-number-truthy" data-foobar={1} />
-<span id="data-attr-number-falsy" data-foobar={0} />
-
-<span id="normal-attr-true" foobar={true} />
-<span id="normal-attr-false" foobar={false} />
-<span id="normal-attr-string-truthy" foobar={'foo'} />
-<span id="normal-attr-string-falsy" foobar={''} />
-<span id="normal-attr-number-truthy" foobar={1} />
-<span id="normal-attr-number-falsy" foobar={0} />
-
+<span id="false-str" attr="false" />
+<span id="true-str" attr="true" />
+<span id="true" attr={true} />
+<span id="false" attr={false} />
+<span id="empty" attr="" />
 <span id="null" attr={null} />
 <span id="undefined" attr={undefined} />
-
 <span id="url" attr={"https://example.com/api/og?title=hello&description=somedescription"}/>
 <span id="code" attr={"cmd: echo \"foo\" && echo \"bar\" > /tmp/hello.txt"} />
-
+<!--
+    Per HTML spec, some attributes should be treated as booleans
+    These should always render <span async /> or <span /> (without a string value)
+-->
+<span id='html-boolean' async />
+<span id='html-boolean-true' async={true} />
+<span id='html-boolean-false' async={false} />
 <!--
     Other attributes should be treated as string enums
     These should always render <span draggable="true" /> or <span draggable="false" />

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

@@ -57,8 +57,8 @@ describe('MDX - Vite env vars', () => {
 		const dataAttrDump = document.querySelector('[data-env-dump]');
 		assert.notEqual(dataAttrDump, null);
 
-		assert.equal(dataAttrDump.getAttribute('data-env-prod'), 'true');
-		assert.equal(dataAttrDump.getAttribute('data-env-dev'), 'false');
+		assert.notEqual(dataAttrDump.getAttribute('data-env-prod'), null);
+		assert.equal(dataAttrDump.getAttribute('data-env-dev'), null);
 		assert.equal(dataAttrDump.getAttribute('data-env-base-url'), '/');
 		assert.equal(dataAttrDump.getAttribute('data-env-mode'), 'production');
 	});