fix(engine): do not render empty string attr values (#4748)

nolanlawson · web-flow · commit 1bb0bfa6047e · 2024-10-31T12:30:05.000-07:00
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-empty-string/error.txt b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-empty-string/error.txt
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-empty-string/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-empty-string/expected.html
@@ -0,0 +1,10 @@
+<x-test>
+  <template shadowrootmode="open">
+    <div data-foo>
+    </div>
+    <div data-foo>
+    </div>
+    <div data-foo>
+    </div>
+  </template>
+</x-test>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-empty-string/index.js b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-empty-string/index.js
@@ -0,0 +1,3 @@
+export const tagName = 'x-test';
+export { default } from 'x/test';
+export * from 'x/test';
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-empty-string/modules/x/test/test.html b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-empty-string/modules/x/test/test.html
@@ -0,0 +1,5 @@
+<template>
+    <div data-foo></div>
+    <div data-foo=''></div>
+    <div data-foo=""></div>
+</template>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-empty-string/modules/x/test/test.js b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-empty-string/modules/x/test/test.js
@@ -0,0 +1,4 @@
+import { LightningElement } from 'lwc';
+
+export default class extends LightningElement {
+}
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/known-boolean-attributes/correct-tag-attr-pair/empty-string/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/known-boolean-attributes/correct-tag-attr-pair/empty-string/expected.html
@@ -1,57 +1,57 @@
 <x-component>
   <template shadowrootmode="open">
-    <button autofocus="">
+    <button autofocus>
     </button>
-    <input autofocus="">
-    <keygen autofocus="">
-    <select autofocus="">
+    <input autofocus>
+    <keygen autofocus>
+    <select autofocus>
     </select>
-    <textarea autofocus="">
+    <textarea autofocus>
     </textarea>
-    <audio autoplay="">
+    <audio autoplay>
     </audio>
-    <video autoplay="">
+    <video autoplay>
     </video>
-    <input checked="">
-    <button disabled="">
+    <input checked>
+    <button disabled>
     </button>
-    <fieldset disabled="">
+    <fieldset disabled>
     </fieldset>
-    <input disabled="">
-    <optgroup disabled="">
+    <input disabled>
+    <optgroup disabled>
     </optgroup>
-    <select disabled="">
+    <select disabled>
     </select>
-    <textarea disabled="">
+    <textarea disabled>
     </textarea>
-    <button formnovalidate="">
+    <button formnovalidate>
     </button>
-    <audio loop="">
+    <audio loop>
     </audio>
-    <video loop="">
+    <video loop>
     </video>
-    <input multiple="">
-    <select multiple="">
+    <input multiple>
+    <select multiple>
     </select>
-    <audio muted="">
+    <audio muted>
     </audio>
-    <video muted="">
+    <video muted>
     </video>
-    <form novalidate="">
+    <form novalidate>
     </form>
-    <details open="">
+    <details open>
     </details>
-    <input readonly="">
-    <textarea readonly="">
+    <input readonly>
+    <textarea readonly>
     </textarea>
-    <input required="">
-    <select required="">
+    <input required>
+    <select required>
     </select>
-    <textarea required="">
+    <textarea required>
     </textarea>
-    <ol reversed="">
+    <ol reversed>
     </ol>
-    <option selected="">
+    <option selected>
     </option>
   </template>
 </x-component>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/known-boolean-attributes/default-def-html-attributes/static/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/known-boolean-attributes/default-def-html-attributes/static/expected.html
@@ -20,21 +20,21 @@
     </div>
     <div title>
     </div>
-    <div accesskey="">
+    <div accesskey>
     </div>
-    <div dir="">
+    <div dir>
     </div>
-    <div draggable="">
+    <div draggable>
     </div>
-    <div hidden="">
+    <div hidden>
     </div>
     <div id>
     </div>
-    <div lang="">
+    <div lang>
     </div>
     <div spellcheck="true">
     </div>
-    <div title="">
+    <div title>
     </div>
     <div accesskey="   ">
     </div>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/known-boolean-attributes/incorrect-tag-attr-pair/static/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/known-boolean-attributes/incorrect-tag-attr-pair/static/expected.html
@@ -4,9 +4,9 @@
     </div>
     <div formnovalidate>
     </div>
-    <div autofocus="">
+    <div autofocus>
     </div>
-    <div formnovalidate="">
+    <div formnovalidate>
     </div>
     <div autofocus="   ">
     </div>
diff --git a/packages/@lwc/template-compiler/src/__tests__/fixtures/attributes/boolean-attribute/hidden-global-attr/expected.js b/packages/@lwc/template-compiler/src/__tests__/fixtures/attributes/boolean-attribute/hidden-global-attr/expected.js
@@ -3,7 +3,7 @@ import _implicitScopedStylesheets from "./hidden-global-attr.scoped.css?scoped=t
 import _xFoo from "x/foo";
 import { freezeTemplate, parseFragment, registerTemplate } from "lwc";
 const $fragment1 = parseFragment`<p hidden${3}>boolean present</p>`;
-const $fragment2 = parseFragment`<p hidden=""${3}>empty string, should be true</p>`;
+const $fragment2 = parseFragment`<p hidden${3}>empty string, should be true</p>`;
 const $fragment3 = parseFragment`<p hidden="other than true"${3}>string value, should be true</p>`;
 const $fragment4 = parseFragment`<p${"a0:hidden"}${3}>computed value, should be resolved in component</p>`;
 const $fragment5 = parseFragment`<p hidden="3"${3}>integer value, should be true</p>`;
diff --git a/packages/@lwc/template-compiler/src/__tests__/fixtures/attributes/boolean-attribute/required/expected.js b/packages/@lwc/template-compiler/src/__tests__/fixtures/attributes/boolean-attribute/required/expected.js
@@ -3,7 +3,7 @@ import _implicitScopedStylesheets from "./required.scoped.css?scoped=true";
 import _xFoo from "x/foo";
 import { freezeTemplate, parseFragment, registerTemplate } from "lwc";
 const $fragment1 = parseFragment`<input required value="boolean present"${3}>`;
-const $fragment2 = parseFragment`<input required="" value="empty string"${3}>`;
+const $fragment2 = parseFragment`<input required value="empty string"${3}>`;
 const $fragment3 = parseFragment`<input required="other than true" value="string value"${3}>`;
 const $fragment4 = parseFragment`<input${"a0:required"} value="computed value"${3}>`;
 const $fragment5 = parseFragment`<input required="3" value="integer value"${3}>`;
diff --git a/packages/@lwc/template-compiler/src/__tests__/fixtures/attributes/error-empty-value/expected.js b/packages/@lwc/template-compiler/src/__tests__/fixtures/attributes/error-empty-value/expected.js
@@ -2,7 +2,7 @@ import _implicitStylesheets from "./error-empty-value.css";
 import _implicitScopedStylesheets from "./error-empty-value.scoped.css?scoped=true";
 import _fooBar from "foo/bar";
 import { freezeTemplate, parseFragment, registerTemplate } from "lwc";
-const $fragment1 = parseFragment`<p title=""${3}></p>`;
+const $fragment1 = parseFragment`<p title${3}></p>`;
 const stc0 = {
   props: {
     content: "",
diff --git a/packages/@lwc/template-compiler/src/codegen/static-element-serializer.ts b/packages/@lwc/template-compiler/src/codegen/static-element-serializer.ts
@@ -121,13 +121,21 @@ function serializeAttrs(element: Element, codeGen: CodeGen): string {
             // "boolean" format (e.g. `<div id>`) as the empty string, which is semantically equivalent.
             const needsPlaceholder = hasExpression || hasSvgUseHref || needsScoping;
 
-            // Inject a placeholder where the staticPartId will go when an expression occurs.
-            // This is only needed for SSR to inject the expression value during serialization.
-            attrs.push(
-                needsPlaceholder
-                    ? `\${"${v}"}`
-                    : ` ${escapedAttributeName}="${htmlEscape(v, true)}"`
-            );
+            let nameAndValue;
+            if (needsPlaceholder) {
+                // Inject a placeholder where the staticPartId will go when an expression occurs.
+                // This is only needed for SSR to inject the expression value during serialization.
+                nameAndValue = `\${"${v}"}`;
+            } else if (v === '') {
+                // In HTML, there is no difference between the empty string value (`<div foo="">`) and "boolean true"
+                // (`<div foo>`). They are both parsed identically, and the DOM treats them the same (`getAttribute`
+                // returns the empty string). Here we prefer the shorter format.
+                // https://html.spec.whatwg.org/multipage/introduction.html#a-quick-introduction-to-html:syntax-attributes
+                nameAndValue = ` ${escapedAttributeName}`;
+            } else {
+                nameAndValue = ` ${escapedAttributeName}="${htmlEscape(v, true)}"`;
+            }
+            attrs.push(nameAndValue);
         } else {
             attrs.push(` ${escapedAttributeName}`);
         }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+export const tagName = 'x-test';`
	`2`	`+export { default } from 'x/test';`
	`3`	`+export * from 'x/test';`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +import { LightningElement } from 'lwc';
++
 +export default class extends LightningElement {
 +}