fix(ssr): make internals non-configurable (#5208)

* fix(ssr): make internals non-configurable * test(ssr): add check that internals are safe * chore: remove rogue file * chore: use correct file extension for test output
salesforce · Feb 10, 2025 · f6b600f · f6b600f
1 parent d6db45d
commit f6b600f
Show file tree

Hide file tree

Showing 9 changed files with 125 additions and 44 deletions.
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/config.json b/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/config.json
@@ -0,0 +1,6 @@
+{
+    "ssrFiles": {
+        "error": "error-ssr.txt",
+        "expected": "expected-ssr.html"
+    }
+}
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/error-ssr.txt b/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/error-ssr.txt
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/error.txt b/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/error.txt
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/expected-ssr.html b/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/expected-ssr.html
@@ -0,0 +1,16 @@
+<x-component>
+  <template shadowrootmode="open">
+    <p>
+      this only matters for ssr-compiler; engine-server does not have the same internals
+    </p>
+    <p>
+      generateMarkup overridden: false
+    </p>
+    <p>
+      tmpl overridden: false
+    </p>
+    <p>
+      public props overridden: false
+    </p>
+  </template>
+</x-component>
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/expected.html
diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/index.js b/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/index.js
@@ -0,0 +1,3 @@
+export const tagName = 'x-component';
+export { default } from 'x/component';
+export * from 'x/component';
diff --git a/...lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.html b/...lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.html
@@ -0,0 +1,6 @@
+<template>
+    <p>this only matters for ssr-compiler; engine-server does not have the same internals</p>
+    <p>generateMarkup overridden: {isGenerateMarkupOverridden}</p>
+    <p>tmpl overridden: {isTmplOverridden}</p>
+    <p>public props overridden: {isPublicPropsOverridden}</p>
+</template>
diff --git a/.../@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.js b/.../@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.js
@@ -0,0 +1,24 @@
+import { LightningElement } from 'lwc';
+import { SYMBOL__DEFAULT_TEMPLATE, SYMBOL__GENERATE_MARKUP } from '@lwc/ssr-runtime';
+
+const myGenerateMarkup = () => '<p>hili</p>';
+const myPublicProperties = ['philip'];
+const myTmpl = () => {
+    throw new Error('PHILIP');
+};
+
+export default class Component extends LightningElement {
+    static [SYMBOL__DEFAULT_TEMPLATE] = myTmpl;
+    static [SYMBOL__GENERATE_MARKUP] = myGenerateMarkup;
+    static ['__lwcPublicProperties__'] = myPublicProperties;
+
+    get isGenerateMarkupOverridden() {
+        return Component[SYMBOL__DEFAULT_TEMPLATE] === myGenerateMarkup;
+    }
+    get isTmplOverridden() {
+        return Component[SYMBOL__GENERATE_MARKUP] === myTmpl;
+    }
+    get isPublicPropsOverridden() {
+        return Component['__lwcPublicProperties__'] === myPublicProperties;
+    }
+}
diff --git a/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts b/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts
@@ -20,7 +20,14 @@ const bGenerateMarkup = esTemplate`
     const __lwcPublicProperties__ = new Set(${/*public properties*/ is.arrayExpression}.concat(__lwcSuperPublicProperties__));
     const __lwcPrivateProperties__ = new Set(${/*private properties*/ is.arrayExpression});
 
-    ${/* component class */ 0}[__SYMBOL__GENERATE_MARKUP] = async function* generateMarkup(
+    Object.defineProperty(
+    ${/* component class */ 0},
+    __SYMBOL__GENERATE_MARKUP,
+    {
+        configurable: false,
+        enumerable: false,
+        writable: false,
+        value: async function* generateMarkup(
             tagName, 
             props, 
             attrs, 
@@ -30,57 +37,76 @@ const bGenerateMarkup = esTemplate`
             parent, 
             scopeToken,
             contextfulParent
-    ) {
-        tagName = tagName ?? ${/*component tag name*/ is.literal};
-        attrs = attrs ?? Object.create(null);
-        props = props ?? Object.create(null);
-        const instance = new ${/* Component class */ 0}({
-            tagName: tagName.toUpperCase(),
-        });
+        ) {
+            tagName = tagName ?? ${/*component tag name*/ is.literal};
+            attrs = attrs ?? Object.create(null);
+            props = props ?? Object.create(null);
+            const instance = new ${/* Component class */ 0}({
+                tagName: tagName.toUpperCase(),
+            });
 
-        __establishContextfulRelationship(contextfulParent, instance);
-        ${/*connect wire*/ is.statement}
+            __establishContextfulRelationship(contextfulParent, instance);
+            ${/*connect wire*/ is.statement}
 
-        instance[__SYMBOL__SET_INTERNALS](
-            props,
-            attrs,
-            __lwcPublicProperties__,
-            __lwcPrivateProperties__,
-        );
-        instance.isConnected = true;
-        if (instance.connectedCallback) {
-            __mutationTracker.enable(instance);
-            instance.connectedCallback();
-            __mutationTracker.disable(instance);
-        }
-        // 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*/ 0}[__SYMBOL__DEFAULT_TEMPLATE] ?? __fallbackTmpl;
-        yield \`<\${tagName}\`;
+            instance[__SYMBOL__SET_INTERNALS](
+                props,
+                attrs,
+                __lwcPublicProperties__,
+                __lwcPrivateProperties__,
+            );
+            instance.isConnected = true;
+            if (instance.connectedCallback) {
+                __mutationTracker.enable(instance);
+                instance.connectedCallback();
+                __mutationTracker.disable(instance);
+            }
+            // 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*/ 0}[__SYMBOL__DEFAULT_TEMPLATE] ?? __fallbackTmpl;
+            yield \`<\${tagName}\`;
 
-        const hostHasScopedStylesheets =
-            tmplFn.hasScopedStylesheets ||
-            hasScopedStaticStylesheets(${/*component class*/ 0});
-        const hostScopeToken = hostHasScopedStylesheets ? tmplFn.stylesheetScopeToken + "-host" : undefined;
+            const hostHasScopedStylesheets =
+                tmplFn.hasScopedStylesheets ||
+                hasScopedStaticStylesheets(${/*component class*/ 0});
+            const hostScopeToken = hostHasScopedStylesheets ? tmplFn.stylesheetScopeToken + "-host" : undefined;
 
-        yield* __renderAttrs(instance, attrs, hostScopeToken, scopeToken);
-        yield '>';
-        yield* tmplFn(
-            shadowSlottedContent,
-            lightSlottedContent,
-            scopedSlottedContent,
-            ${/*component class*/ 0},
-            instance
-        );
-        yield \`</\${tagName}>\`;
-    }
-    ${/* component class */ 0}.__lwcPublicProperties__ = __lwcPublicProperties__;
+            yield* __renderAttrs(instance, attrs, hostScopeToken, scopeToken);
+            yield '>';
+            yield* tmplFn(
+                shadowSlottedContent,
+                lightSlottedContent,
+                scopedSlottedContent,
+                ${/*component class*/ 0},
+                instance
+            );
+            yield \`</\${tagName}>\`;
+        }
+    });
+    Object.defineProperty(
+        ${/* component class */ 0},
+        '__lwcPublicProperties__',
+        {
+            configurable: false,
+            enumerable: false,
+            writable: false,
+            value: __lwcPublicProperties__
+        }
+    );
 `<[Statement]>;
 
 const bExposeTemplate = esTemplate`
     if (${/*template*/ is.identifier}) {
-        ${/* component class */ is.identifier}[__SYMBOL__DEFAULT_TEMPLATE] = ${/*template*/ 0}
+        Object.defineProperty(
+            ${/* component class */ is.identifier},
+            __SYMBOL__DEFAULT_TEMPLATE,
+            {
+                configurable: false,
+                enumerable: false,
+                writable: false,
+                value: ${/*template*/ 0}
+            }
+        );
     }
 `<IfStatement>;