From f6b600f3433283c3b388730524e5fc73abe4035d Mon Sep 17 00:00:00 2001 From: Will Harney <62956339+wjhsf@users.noreply.github.com> Date: Mon, 10 Feb 2025 17:05:24 -0500 Subject: [PATCH] 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 --- .../fixtures/ssr-internals/config.json | 6 + .../fixtures/ssr-internals/error-ssr.txt | 0 .../fixtures/ssr-internals/error.txt | 0 .../fixtures/ssr-internals/expected-ssr.html | 16 +++ .../fixtures/ssr-internals/expected.html | 0 .../__tests__/fixtures/ssr-internals/index.js | 3 + .../modules/x/component/component.html | 6 + .../modules/x/component/component.js | 24 ++++ .../src/compile-js/generate-markup.ts | 114 +++++++++++------- 9 files changed, 125 insertions(+), 44 deletions(-) create mode 100644 packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/config.json create mode 100644 packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/error-ssr.txt create mode 100644 packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/error.txt create mode 100644 packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/expected-ssr.html create mode 100644 packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/expected.html create mode 100644 packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/index.js create mode 100644 packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.html create mode 100644 packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.js 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 new file mode 100644 index 0000000000..59676736cb --- /dev/null +++ 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 new file mode 100644 index 0000000000..e69de29bb2 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 new file mode 100644 index 0000000000..e69de29bb2 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 new file mode 100644 index 0000000000..1008a98dec --- /dev/null +++ b/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/expected-ssr.html @@ -0,0 +1,16 @@ + + + \ No newline at end of file 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 new file mode 100644 index 0000000000..e69de29bb2 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 new file mode 100644 index 0000000000..cd34090d2c --- /dev/null +++ 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/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.html b/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.html new file mode 100644 index 0000000000..cb84fb081f --- /dev/null +++ b/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.html @@ -0,0 +1,6 @@ + diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.js b/packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.js new file mode 100644 index 0000000000..a39cc98b8f --- /dev/null +++ b/packages/@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 = () => '

hili

'; +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 index 7d7f01c71d..137fe5a589 100644 --- 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 \`\`; - } - ${/* component class */ 0}.__lwcPublicProperties__ = __lwcPublicProperties__; + yield* __renderAttrs(instance, attrs, hostScopeToken, scopeToken); + yield '>'; + yield* tmplFn( + shadowSlottedContent, + lightSlottedContent, + scopedSlottedContent, + ${/*component class*/ 0}, + instance + ); + yield \`\`; + } + }); + 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} + } + ); } `;