feat(ssr): implement fixes for known issues (#5188)

divmain · web-flow · commit 7dc34fca220f · 2025-02-04T12:50:13.000-05:00
* chore: add test for api-decorated super field

* fix: api-decorated fields on non-LightningElement super class

* chore: cleanup previous commit

* chore: add failing test for subclass edge case

* fix: ssr superclass edge case

* chore: augment test with truthy/falsey if:true blocks

* fix: legacy if-blocks for truthy/falsey conditions in SSRv2

* fix: behavior of SSR compiler when DiagnosticLevel.Fatal message is encountered

* feat: implement ClassList#length
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/error.txt b/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/error.txt
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/expected.html
@@ -0,0 +1,24 @@
+<x-parent>
+  <template shadowrootmode="open">
+    <x-child>
+      <template shadowrootmode="open">
+        <p>
+          Super: super string theory
+        </p>
+        <p>
+          Untalented: untalented string theory
+        </p>
+      </template>
+    </x-child>
+    <x-child>
+      <template shadowrootmode="open">
+        <p>
+          Super: field api value
+        </p>
+        <p>
+          Untalented: field api value
+        </p>
+      </template>
+    </x-child>
+  </template>
+</x-parent>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/index.js b/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/index.js
@@ -0,0 +1,3 @@
+export const tagName = 'x-parent';
+export { default } from 'x/parent';
+export * from 'x/parent';
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/modules/x/child/child.html b/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/modules/x/child/child.html
@@ -0,0 +1,4 @@
+<template>
+    <p>Super: {superField}</p>
+    <p>Untalented: {untalentedField}</p>
+</template>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/modules/x/child/child.js b/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/modules/x/child/child.js
@@ -0,0 +1,6 @@
+import { api } from 'lwc';
+import SuperChild from 'x/superChild';
+
+export default class Child extends SuperChild {
+    @api untalentedField;
+}
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/modules/x/parent/parent.html b/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/modules/x/parent/parent.html
@@ -0,0 +1,4 @@
+<template>
+    <x-child untalented-field="untalented string theory" super-field="super string theory"></x-child>
+    <x-child untalented-field={fieldApiValue} super-field={fieldApiValue}></x-child>
+</template>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/modules/x/parent/parent.js b/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/modules/x/parent/parent.js
@@ -0,0 +1,5 @@
+import { LightningElement } from 'lwc';
+
+export default class Parent extends LightningElement {
+    fieldApiValue = 'field api value';
+}
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/modules/x/superChild/superChild.js b/packages/@lwc/engine-server/src/__tests__/fixtures/api/inherited/modules/x/superChild/superChild.js
@@ -0,0 +1,5 @@
+import { LightningElement, api } from 'lwc';
+
+export default class SuperChild extends LightningElement {
+    @api superField;
+}
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/getter-class-list/modules/x/getter-class-list/getter-class-list.js b/packages/@lwc/engine-server/src/__tests__/fixtures/getter-class-list/modules/x/getter-class-list/getter-class-list.js
@@ -9,5 +9,6 @@ export default class GetterClassList extends LightningElement {
         classList.remove('b');
 
         expect(this.getAttribute('class')).toBe('a c d-e');
+        expect(() => this.classList.length).not.toThrow();
     }
 }
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/if-block/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/if-block/expected.html
@@ -1,5 +1,22 @@
 <x-if-block>
   <template shadowrootmode="open">
-    I am rendered!
+    <h1>
+      True!
+    </h1>
+    <p>
+      I am rendered because I am true to myself!
+    </p>
+    <p>
+      I am rendered because I am true enough!
+    </p>
+    <h1>
+      False!
+    </h1>
+    <p>
+      I am rendered because I am very false!
+    </p>
+    <p>
+      I am rendered because I am untrustworthy!
+    </p>
   </template>
 </x-if-block>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/if-block/modules/x/if-block/if-block.html b/packages/@lwc/engine-server/src/__tests__/fixtures/if-block/modules/x/if-block/if-block.html
@@ -1,8 +1,28 @@
 <template>
+    <h1>True!</h1>
     <template if:true={isTrue}>
-        I am rendered!
+        <p>I am rendered because I am true to myself!</p>
     </template>
     <template if:true={isFalse}>
-        I am not rendered!
+        <p>I am not rendered because I am very false!</p>
+    </template>
+    <template if:true={isTruthy}>
+        <p>I am rendered because I am true enough!</p>
+    </template>
+    <template if:true={isFalsey}>
+        <p>I am not rendered because I am untrustworthy!</p>
+    </template>
+    <h1>False!</h1>
+    <template if:false={isTrue}>
+        <p>I am not rendered because I am true to myself!</p>
+    </template>
+    <template if:false={isFalse}>
+        <p>I am rendered because I am very false!</p>
+    </template>
+    <template if:false={isTruthy}>
+        <p>I am not rendered because I am true enough!</p>
+    </template>
+    <template if:false={isFalsey}>
+        <p>I am rendered because I am untrustworthy!</p>
     </template>
 </template>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/if-block/modules/x/if-block/if-block.js b/packages/@lwc/engine-server/src/__tests__/fixtures/if-block/modules/x/if-block/if-block.js
@@ -3,4 +3,6 @@ import { LightningElement, api } from 'lwc';
 export default class IfBlock extends LightningElement {
     isTrue = true;
     isFalse = false;
+    isTruthy = 'truthy value';
+    isFalsey = null;
 }
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/error.txt b/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/error.txt
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/expected.html
@@ -0,0 +1,5 @@
+<x-cmp>
+  <template shadowrootmode="open">
+    Value: foobar
+  </template>
+</x-cmp>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/index.js b/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/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/non-lwc-class/modules/x/cmp/cmp.html b/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/modules/x/cmp/cmp.html
@@ -0,0 +1,4 @@
+<template>
+    Value: {foo.value}
+    <!-- Dude -->
+</template>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/modules/x/cmp/cmp.js b/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/modules/x/cmp/cmp.js
@@ -0,0 +1,6 @@
+import { LightningElement } from 'lwc';
+import valueFromNonLightningElement from 'x/notLightningElement';
+
+export default class Cmp extends LightningElement {
+    foo = valueFromNonLightningElement;
+}
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/modules/x/notLightningElement/notLightningElement.js b/packages/@lwc/engine-server/src/__tests__/fixtures/non-lwc-class/modules/x/notLightningElement/notLightningElement.js
@@ -0,0 +1,19 @@
+class SuperClass {
+    constructor() {
+        this._value = 'foo';
+    }
+}
+
+class NotLightningElement extends SuperClass {
+    // export default class NotLightningElement extends SuperClass {
+    constructor() {
+        super();
+        this._value += 'bar';
+    }
+
+    get value() {
+        return this._value;
+    }
+}
+
+export default new NotLightningElement();
diff --git a/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts b/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts
@@ -16,8 +16,9 @@ import type { ComponentMetaState } from './types';
 
 const bGenerateMarkup = esTemplate`
     // These variables may mix with component-authored variables, so should be reasonably unique
-    const __lwcPublicProperties__ = new Set(${/*api*/ is.arrayExpression});
-    const __lwcPrivateProperties__ = new Set(${/*private fields*/ is.arrayExpression});
+    const __lwcSuperPublicProperties__ = Array.from(Object.getPrototypeOf(${/* Component class */ is.identifier})?.__lwcPublicProperties__?.values?.() ?? []);
+    const __lwcPublicProperties__ = new Set(${/*public properties*/ is.arrayExpression}.concat(__lwcSuperPublicProperties__));
+    const __lwcPrivateProperties__ = new Set(${/*private properties*/ is.arrayExpression});
 
     async function* generateMarkup(
             tagName, 
@@ -33,7 +34,7 @@ const bGenerateMarkup = esTemplate`
         tagName = tagName ?? ${/*component tag name*/ is.literal};
         attrs = attrs ?? Object.create(null);
         props = props ?? Object.create(null);
-        const instance = new ${/* Component class */ is.identifier}({
+        const instance = new ${/* Component class */ 0}({
             tagName: tagName.toUpperCase(),
         });
 
@@ -55,12 +56,12 @@ const bGenerateMarkup = esTemplate`
         // If a render() function is defined on the class or any of its superclasses, then that takes priority.
         // Next, if the class or any of its superclasses has an implicitly-associated template, then that takes
         // second priority (e.g. a foo.html file alongside a foo.js file). Finally, there is a fallback empty template.
-        const tmplFn = instance.render?.() ?? ${/*component class*/ 3}[__SYMBOL__DEFAULT_TEMPLATE] ?? __fallbackTmpl;
+        const tmplFn = instance.render?.() ?? ${/*component class*/ 0}[__SYMBOL__DEFAULT_TEMPLATE] ?? __fallbackTmpl;
         yield \`<\${tagName}\`;
 
         const hostHasScopedStylesheets =
             tmplFn.hasScopedStylesheets ||
-            hasScopedStaticStylesheets(${/*component class*/ 3});
+            hasScopedStaticStylesheets(${/*component class*/ 0});
         const hostScopeToken = hostHasScopedStylesheets ? tmplFn.stylesheetScopeToken + "-host" : undefined;
 
         yield* __renderAttrs(instance, attrs, hostScopeToken, scopeToken);
@@ -69,12 +70,13 @@ const bGenerateMarkup = esTemplate`
             shadowSlottedContent,
             lightSlottedContent,
             scopedSlottedContent,
-            ${/*component class*/ 3},
+            ${/*component class*/ 0},
             instance
         );
         yield \`</\${tagName}>\`;
     }
-    ${/* component class */ 3}[__SYMBOL__GENERATE_MARKUP] = generateMarkup;
+    ${/* component class */ 0}[__SYMBOL__GENERATE_MARKUP] = generateMarkup;
+    ${/* component class */ 0}.__lwcPublicProperties__ = __lwcPublicProperties__;
 `<[Statement]>;
 
 const bExposeTemplate = esTemplate`
@@ -141,10 +143,10 @@ export function addGenerateMarkupFunction(
     );
     program.body.push(
         ...bGenerateMarkup(
+            classIdentifier,
             b.arrayExpression(publicProperties.map(b.literal)),
             b.arrayExpression(privateProperties.map(b.literal)),
             defaultTagName,
-            classIdentifier,
             connectWireAdapterCode
         )
     );
diff --git a/packages/@lwc/ssr-compiler/src/compile-js/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/index.ts
@@ -215,7 +215,13 @@ const visitors: Visitors = {
                 break;
         }
     },
-    Super(path, _state) {
+    Super(path, state) {
+        // If we mutate any super calls that are piped through this compiler, then we'll be
+        // inadvertently mutating things like Wire adapters.
+        if (!state.isLWC) {
+            return;
+        }
+
         const parentFn = path.getFunctionParent();
         if (
             parentFn &&
diff --git a/packages/@lwc/ssr-compiler/src/compile-template/index.ts b/packages/@lwc/ssr-compiler/src/compile-template/index.ts
@@ -8,7 +8,6 @@
 import { generate } from 'astring';
 import { is, builders as b } from 'estree-toolkit';
 import { parse, type Config as TemplateCompilerConfig } from '@lwc/template-compiler';
-import { DiagnosticLevel } from '@lwc/errors';
 import { esTemplate } from '../estemplate';
 import { getStylesheetImports } from '../compile-js/stylesheets';
 import { addScopeTokenDeclarations } from '../compile-js/stylesheet-scope-token';
@@ -93,19 +92,15 @@ export default function compileTemplate(
         experimentalDynamicDirective: options.experimentalDynamicDirective,
     });
     if (!root || warnings.length) {
-        let fatal = !root;
         for (const warning of warnings) {
             // eslint-disable-next-line no-console
             console.error('Cannot compile:', warning.message);
-            if (
-                warning.level === DiagnosticLevel.Fatal ||
-                warning.level === DiagnosticLevel.Error
-            ) {
-                fatal = true;
-            }
         }
-        // || !root is just used here to make TypeScript happy
-        if (fatal || !root) {
+        // The legacy SSR implementation would not bail from compilation even if a
+        // DiagnosticLevel.Fatal error was encountered. It would only fail if the
+        // template parser failed to return a root node. That behavior is duplicated
+        // here.
+        if (!root) {
             throw new Error('Template compilation failure; see warnings in the console.');
         }
     }
@@ -115,7 +110,7 @@ export default function compileTemplate(
     )?.value?.value;
     const experimentalComplexExpressions = Boolean(options.experimentalComplexExpressions);
 
-    const { addImport, getImports, statements } = templateIrToEsTree(root!, {
+    const { addImport, getImports, statements } = templateIrToEsTree(root, {
         preserveComments,
         experimentalComplexExpressions,
     });
diff --git a/packages/@lwc/ssr-compiler/src/compile-template/transformers/legacyIf.ts b/packages/@lwc/ssr-compiler/src/compile-template/transformers/legacyIf.ts
@@ -17,15 +17,12 @@ export const LegacyIf: Transformer<IrIf> = function If(node, cxt) {
     const { modifier: trueOrFalseAsStr, condition, children } = node;
 
     const trueOrFalse = trueOrFalseAsStr === 'true';
-    // FIXME: Does engine-server actually do triple-equals here?
-    const comparison = b.binaryExpression(
-        '===',
-        b.literal(trueOrFalse),
-        expressionIrToEs(condition, cxt)
-    );
+    const test = trueOrFalse
+        ? expressionIrToEs(condition, cxt)
+        : b.unaryExpression('!', expressionIrToEs(condition, cxt));
 
     const childStatements = irChildrenToEs(children, cxt);
     const block = b.blockStatement(optimizeAdjacentYieldStmts(childStatements));
 
-    return [b.ifStatement(comparison, block)];
+    return [b.ifStatement(test, block)];
 };
diff --git a/packages/@lwc/ssr-runtime/src/class-list.ts b/packages/@lwc/ssr-runtime/src/class-list.ts
@@ -92,6 +92,11 @@ export class ClassList implements DOMTokenList {
         return this.el.className;
     }
 
+    get length(): number {
+        const currentClassNameStr = this.el.className ?? '';
+        return currentClassNameStr.split(MULTI_SPACE).length;
+    }
+
     // Stubs to satisfy DOMTokenList interface
     [index: number]: never; // Can't implement arbitrary index getters without a proxy
     item(_index: number): string | null {
@@ -106,7 +111,4 @@ export class ClassList implements DOMTokenList {
     ): void {
         throw new Error('Method "forEach" not implemented.');
     }
-    get length(): number {
-        throw new Error('Property "length" not implemented.');
-    }
 }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+export const tagName = 'x-parent';`
	`2`	`+export { default } from 'x/parent';`
	`3`	`+export * from 'x/parent';`
Original file line number	Diff line number	Diff line change
`@@ -9,5 +9,6 @@ export default class GetterClassList extends LightningElement {`
`9`	`9`	`classList.remove('b');`
`10`	`10`
`11`	`11`	`expect(this.getAttribute('class')).toBe('a c d-e');`
	`12`	`+ expect(() => this.classList.length).not.toThrow();`
`12`	`13`	`}`
`13`	`14`	`}`