fix(hydration): only ignore mutated host attributes (#4385)

nolanlawson · web-flow · commit 4d13710821c4 · 2024-07-17T08:09:30.000-07:00
diff --git a/packages/@lwc/engine-core/src/framework/hydration.ts b/packages/@lwc/engine-core/src/framework/hydration.ts
@@ -20,6 +20,7 @@ import {
     APIFeature,
     isAPIFeatureEnabled,
     isFalse,
+    StringSplit,
 } from '@lwc/shared';
 
 import { logError, logWarn } from '../shared/logger';
@@ -165,9 +166,11 @@ function getValidationPredicate(
     optOutStaticProp: string[] | true | undefined
 ): AttrValidationPredicate {
     // `data-lwc-host-mutated` is a special attribute added by the SSR engine itself,
-    // which does the same thing as an explicit `static validationOptOut = true`.
-    if (renderer.getAttribute(elm, 'data-lwc-host-mutated') === '') {
-        return (_attrName: string) => false;
+    // which does the same thing as an explicit `static validationOptOut = ['attr1', 'attr2']`.
+    const hostMutatedValue = renderer.getAttribute(elm, 'data-lwc-host-mutated');
+    if (isString(hostMutatedValue)) {
+        const mutatedAttrValues = new Set(StringSplit.call(hostMutatedValue, / /));
+        return (attrName: string) => !mutatedAttrValues.has(attrName);
     }
 
     if (isUndefined(optOutStaticProp)) {
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-and-class-modify/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-and-class-modify/expected.html
@@ -0,0 +1,4 @@
+<x-cmp aria-label="haha" class="yolo woot" data-foo="bar" data-lwc-host-mutated="aria-label class data-foo">
+  <template shadowrootmode="open">
+  </template>
+</x-cmp>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-and-class-modify/index.js b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-and-class-modify/index.js
@@ -0,0 +1,3 @@
+export const tagName = 'x-cmp';
+export { default } from 'x/cmp';
+export * from 'x/cmp';
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-and-class-modify/modules/x/cmp/cmp.html b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-and-class-modify/modules/x/cmp/cmp.html
@@ -0,0 +1,2 @@
+<template>
+</template>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-and-class-modify/modules/x/cmp/cmp.js b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-and-class-modify/modules/x/cmp/cmp.js
@@ -0,0 +1,12 @@
+import { LightningElement } from 'lwc';
+
+export default class extends LightningElement {
+  connectedCallback() {
+    // Modify this component's class and attributes at runtime
+    // We expect a data-lwc-host-mutated attr to be added with the mutated attribute names in unique sorted order
+    this.setAttribute('data-foo', 'bar')
+    this.classList.add('yolo')
+    this.classList.add('woot')
+    this.setAttribute('aria-label', 'haha')
+  }
+}
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-aria-modify/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-aria-modify/expected.html
@@ -1,4 +1,4 @@
-<x-cmp aria-activedescendant="foo" aria-busy="true" data-lwc-host-mutated>
+<x-cmp aria-activedescendant="foo" aria-busy="true" data-lwc-host-mutated="aria-activedescendant aria-busy">
   <template shadowrootmode="open">
   </template>
 </x-cmp>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-modify-uppercase/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-modify-uppercase/expected.html
@@ -0,0 +1,4 @@
+<x-cmp ARIA-LABEL="haha" DATA-FOO="bar" data-lwc-host-mutated="aria-label data-bar data-foo">
+  <template shadowrootmode="open">
+  </template>
+</x-cmp>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-modify-uppercase/index.js b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-modify-uppercase/index.js
@@ -0,0 +1,3 @@
+export const tagName = 'x-cmp';
+export { default } from 'x/cmp';
+export * from 'x/cmp';
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-modify-uppercase/modules/x/cmp/cmp.html b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-modify-uppercase/modules/x/cmp/cmp.html
@@ -0,0 +1,2 @@
+<template>
+</template>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-modify-uppercase/modules/x/cmp/cmp.js b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-modify-uppercase/modules/x/cmp/cmp.js
@@ -0,0 +1,12 @@
+import { LightningElement } from 'lwc';
+
+export default class extends LightningElement {
+  connectedCallback() {
+    // Modify this component's attributes at runtime, using uppercase
+    // We expect a data-lwc-host-mutated attr to be added with the mutated attribute names in unique sorted order,
+    // all lowercase
+    this.setAttribute('DATA-FOO', 'bar')
+    this.setAttribute('ARIA-LABEL', 'haha')
+    this.removeAttribute('dAtA-BaR')
+  }
+}
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/getter-class-list/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/getter-class-list/expected.html
@@ -1,4 +1,4 @@
-<x-getter-class-list class="a c d-e" data-lwc-host-mutated>
+<x-getter-class-list class="a c d-e" data-lwc-host-mutated="class">
   <template shadowrootmode="open">
   </template>
 </x-getter-class-list>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/method-remove-attribute/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/method-remove-attribute/expected.html
@@ -1,4 +1,4 @@
-<x-method-remove-attribute data-a data-c data-lwc-host-mutated>
+<x-method-remove-attribute data-a data-c data-lwc-host-mutated="data-a data-b data-c data-unknown">
   <template shadowrootmode="open">
   </template>
 </x-method-remove-attribute>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/method-set-attribute/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/method-set-attribute/expected.html
@@ -1,4 +1,4 @@
-<x-method-set-attribute data-boolean="true" data-empty-string data-lwc-host-mutated data-null="null" data-number="1" data-override="override" data-string="test">
+<x-method-set-attribute data-boolean="true" data-empty-string data-lwc-host-mutated="data-boolean data-empty-string data-null data-number data-override data-string" data-null="null" data-number="1" data-override="override" data-string="test">
   <template shadowrootmode="open">
   </template>
 </x-method-set-attribute>
diff --git a/packages/@lwc/engine-server/src/renderer.ts b/packages/@lwc/engine-server/src/renderer.ts
@@ -258,7 +258,7 @@ function getAttribute(element: E, name: string, namespace: string | null = null)
 }
 
 function setAttribute(element: E, name: string, value: string, namespace: string | null = null) {
-    reportMutation(element);
+    reportMutation(element, name);
     const attribute = element[HostAttributesKey].find(
         (attr) => attr.name === name && attr[HostNamespaceKey] === namespace
     );
@@ -279,7 +279,7 @@ function setAttribute(element: E, name: string, value: string, namespace: string
 }
 
 function removeAttribute(element: E, name: string, namespace?: string | null) {
-    reportMutation(element);
+    reportMutation(element, name);
     element[HostAttributesKey] = element[HostAttributesKey].filter(
         (attr) => attr.name !== name && attr[HostNamespaceKey] !== namespace
     );
@@ -305,15 +305,15 @@ function getClassList(element: E) {
 
     return {
         add(...names: string[]): void {
-            reportMutation(element);
+            reportMutation(element, 'class');
             const classAttribute = getClassAttribute();
 
             const tokenList = classNameToTokenList(classAttribute.value);
             names.forEach((name) => tokenList.add(name));
             classAttribute.value = tokenListToClassName(tokenList);
         },
         remove(...names: string[]): void {
-            reportMutation(element);
+            reportMutation(element, 'class');
             const classAttribute = getClassAttribute();
 
             const tokenList = classNameToTokenList(classAttribute.value);
diff --git a/packages/@lwc/engine-server/src/utils/mutation-tracking.ts b/packages/@lwc/engine-server/src/utils/mutation-tracking.ts
@@ -10,16 +10,25 @@ const elementsToTrackForMutations: WeakSet<HostElement> = new WeakSet();
 
 const MUTATION_TRACKING_ATTRIBUTE = 'data-lwc-host-mutated';
 
-export function reportMutation(element: HostElement) {
+export function reportMutation(element: HostElement, attributeName: string) {
     if (elementsToTrackForMutations.has(element)) {
-        const hasMutationAttribute = element[HostAttributesKey].find(
+        const existingMutationAttribute = element[HostAttributesKey].find(
             (attr) => attr.name === MUTATION_TRACKING_ATTRIBUTE && attr[HostNamespaceKey] === null
         );
-        if (!hasMutationAttribute) {
+        const attrNameValues = new Set(
+            existingMutationAttribute ? existingMutationAttribute.value.split(' ') : []
+        );
+        attrNameValues.add(attributeName.toLowerCase());
+
+        const newMutationAttributeValue = [...attrNameValues].sort().join(' ');
+
+        if (existingMutationAttribute) {
+            existingMutationAttribute.value = newMutationAttributeValue;
+        } else {
             element[HostAttributesKey].push({
                 name: MUTATION_TRACKING_ATTRIBUTE,
                 [HostNamespaceKey]: null,
-                value: '',
+                value: newMutationAttributeValue,
             });
         }
     }
diff --git a/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/attr-mutated-class-mismatch/index.spec.js b/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/attr-mutated-class-mismatch/index.spec.js
@@ -0,0 +1,35 @@
+export default {
+    props: {
+        ssr: true,
+    },
+    clientProps: {
+        ssr: false,
+    },
+    snapshot(target) {
+        const child = target.shadowRoot.querySelector('x-child');
+        const div = child.shadowRoot.querySelector('div');
+
+        return {
+            child,
+            div,
+        };
+    },
+    test(target, snapshots, consoleCalls) {
+        const snapshotAfterHydration = this.snapshot(target);
+        expect(snapshotAfterHydration.child).not.toBe(snapshots.child);
+        expect(snapshotAfterHydration.div).not.toBe(snapshots.div);
+
+        const { child } = snapshotAfterHydration;
+        expect(child.getAttribute('data-foo')).toBe('bar');
+        expect(child.getAttribute('data-mutatis')).toBe('mutandis');
+        expect(child.getAttribute('class')).toBe('is-client');
+
+        TestUtils.expectConsoleCallsDev(consoleCalls, {
+            warn: [],
+            error: [
+                'Mismatch hydrating element <x-child>: attribute "class" has different values, expected "is-client" but found "is-server"',
+                'Hydration completed with errors.',
+            ],
+        });
+    },
+};
diff --git a/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/attr-mutated-class-mismatch/x/child/child.html b/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/attr-mutated-class-mismatch/x/child/child.html
@@ -0,0 +1,3 @@
+<template>
+    <div>{yolo}</div>
+</template>
diff --git a/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/attr-mutated-class-mismatch/x/child/child.js b/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/attr-mutated-class-mismatch/x/child/child.js
@@ -0,0 +1,9 @@
+import { LightningElement } from 'lwc';
+
+export default class Child extends LightningElement {
+    yolo = 'woot';
+
+    connectedCallback() {
+        this.setAttribute('data-mutatis', 'mutandis');
+    }
+}
diff --git a/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/attr-mutated-class-mismatch/x/main/main.html b/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/attr-mutated-class-mismatch/x/main/main.html
@@ -0,0 +1,3 @@
+<template>
+    <x-child data-foo="bar" class={mismatchingClass}></x-child>
+</template>
diff --git a/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/attr-mutated-class-mismatch/x/main/main.js b/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/attr-mutated-class-mismatch/x/main/main.js
@@ -0,0 +1,9 @@
+import { LightningElement, api } from 'lwc';
+
+export default class Main extends LightningElement {
+    @api ssr;
+
+    get mismatchingClass() {
+        return this.ssr ? 'is-server' : 'is-client';
+    }
+}
diff --git a/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/class-mutated-attr-mismatch/index.spec.js b/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/class-mutated-attr-mismatch/index.spec.js
@@ -0,0 +1,34 @@
+export default {
+    props: {
+        ssr: true,
+    },
+    clientProps: {
+        ssr: false,
+    },
+    snapshot(target) {
+        const child = target.shadowRoot.querySelector('x-child');
+        const div = child.shadowRoot.querySelector('div');
+
+        return {
+            child,
+            div,
+        };
+    },
+    test(target, snapshots, consoleCalls) {
+        const snapshotAfterHydration = this.snapshot(target);
+        expect(snapshotAfterHydration.child).not.toBe(snapshots.child);
+        expect(snapshotAfterHydration.div).not.toBe(snapshots.div);
+
+        const { child } = snapshotAfterHydration;
+        expect(child.getAttribute('class')).toBe('static mutatis');
+        expect(child.getAttribute('data-mismatched-attr')).toBe('is-client');
+
+        TestUtils.expectConsoleCallsDev(consoleCalls, {
+            warn: [],
+            error: [
+                'Mismatch hydrating element <x-child>: attribute "data-mismatched-attr" has different values, expected "is-client" but found "is-server"',
+                'Hydration completed with errors.',
+            ],
+        });
+    },
+};
diff --git a/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/class-mutated-attr-mismatch/x/child/child.html b/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/class-mutated-attr-mismatch/x/child/child.html
@@ -0,0 +1,3 @@
+<template>
+    <div>{yolo}</div>
+</template>
diff --git a/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/class-mutated-attr-mismatch/x/child/child.js b/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/class-mutated-attr-mismatch/x/child/child.js
@@ -0,0 +1,11 @@
+import { LightningElement } from 'lwc';
+
+export default class Child extends LightningElement {
+    yolo = 'woot';
+
+    connectedCallback() {
+        this.classList.add('mutatis');
+        this.classList.add('mutandis');
+        this.classList.remove('mutandis');
+    }
+}
diff --git a/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/class-mutated-attr-mismatch/x/main/main.html b/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/class-mutated-attr-mismatch/x/main/main.html
@@ -0,0 +1,3 @@
+<template>
+    <x-child class="static" data-mismatched-attr={mismatchedAttr}></x-child>
+</template>
diff --git a/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/class-mutated-attr-mismatch/x/main/main.js b/packages/@lwc/integration-karma/test-hydration/mismatches/host-mutation-in-connected-callback/class-mutated-attr-mismatch/x/main/main.js
@@ -0,0 +1,9 @@
+import { LightningElement, api } from 'lwc';
+
+export default class Main extends LightningElement {
+    @api ssr;
+
+    get mismatchedAttr() {
+        return this.ssr ? 'is-server' : 'is-client';
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+export const tagName = 'x-cmp';`
	`2`	`+export { default } from 'x/cmp';`
	`3`	`+export * from 'x/cmp';`