Skip to content

Commit f6b600f

Browse files
wjhsfwjhsf
authored
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
1 parent d6db45d commit f6b600f

File tree

9 files changed

+125
-44
lines changed

9 files changed

+125
-44
lines changed

‎packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/config.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ssrFiles": {
3+
"error": "error-ssr.txt",
4+
"expected": "expected-ssr.html"
5+
}
6+
}

‎packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/error-ssr.txt‎

Whitespace-only changes.

‎packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/error.txt‎

Whitespace-only changes.

‎packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/expected-ssr.html‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<x-component>
2+
<template shadowrootmode="open">
3+
<p>
4+
this only matters for ssr-compiler; engine-server does not have the same internals
5+
</p>
6+
<p>
7+
generateMarkup overridden: false
8+
</p>
9+
<p>
10+
tmpl overridden: false
11+
</p>
12+
<p>
13+
public props overridden: false
14+
</p>
15+
</template>
16+
</x-component>

‎packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/expected.html‎

Whitespace-only changes.

‎packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/index.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export const tagName = 'x-component';
2+
export { default } from 'x/component';
3+
export * from 'x/component';

‎packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.html‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<template>
2+
<p>this only matters for ssr-compiler; engine-server does not have the same internals</p>
3+
<p>generateMarkup overridden: {isGenerateMarkupOverridden}</p>
4+
<p>tmpl overridden: {isTmplOverridden}</p>
5+
<p>public props overridden: {isPublicPropsOverridden}</p>
6+
</template>

‎packages/@lwc/engine-server/src/__tests__/fixtures/ssr-internals/modules/x/component/component.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { LightningElement } from 'lwc';
2+
import { SYMBOL__DEFAULT_TEMPLATE, SYMBOL__GENERATE_MARKUP } from '@lwc/ssr-runtime';
3+
4+
const myGenerateMarkup = () => '<p>hili</p>';
5+
const myPublicProperties = ['philip'];
6+
const myTmpl = () => {
7+
throw new Error('PHILIP');
8+
};
9+
10+
export default class Component extends LightningElement {
11+
static [SYMBOL__DEFAULT_TEMPLATE] = myTmpl;
12+
static [SYMBOL__GENERATE_MARKUP] = myGenerateMarkup;
13+
static ['__lwcPublicProperties__'] = myPublicProperties;
14+
15+
get isGenerateMarkupOverridden() {
16+
return Component[SYMBOL__DEFAULT_TEMPLATE] === myGenerateMarkup;
17+
}
18+
get isTmplOverridden() {
19+
return Component[SYMBOL__GENERATE_MARKUP] === myTmpl;
20+
}
21+
get isPublicPropsOverridden() {
22+
return Component['__lwcPublicProperties__'] === myPublicProperties;
23+
}
24+
}

‎packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts‎

Lines changed: 70 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,14 @@ const bGenerateMarkup = esTemplate`
2020
const __lwcPublicProperties__ = new Set(${/*public properties*/ is.arrayExpression}.concat(__lwcSuperPublicProperties__));
2121
const __lwcPrivateProperties__ = new Set(${/*private properties*/ is.arrayExpression});
2222
23-
${/* component class */ 0}[__SYMBOL__GENERATE_MARKUP] = async function* generateMarkup(
23+
Object.defineProperty(
24+
${/* component class */ 0},
25+
__SYMBOL__GENERATE_MARKUP,
26+
{
27+
configurable: false,
28+
enumerable: false,
29+
writable: false,
30+
value: async function* generateMarkup(
2431
tagName,
2532
props,
2633
attrs,
@@ -30,57 +37,76 @@ const bGenerateMarkup = esTemplate`
3037
parent,
3138
scopeToken,
3239
contextfulParent
33-
) {
34-
tagName = tagName ?? ${/*component tag name*/ is.literal};
35-
attrs = attrs ?? Object.create(null);
36-
props = props ?? Object.create(null);
37-
const instance = new ${/* Component class */ 0}({
38-
tagName: tagName.toUpperCase(),
39-
});
40+
) {
41+
tagName = tagName ?? ${/*component tag name*/ is.literal};
42+
attrs = attrs ?? Object.create(null);
43+
props = props ?? Object.create(null);
44+
const instance = new ${/* Component class */ 0}({
45+
tagName: tagName.toUpperCase(),
46+
});
4047
41-
__establishContextfulRelationship(contextfulParent, instance);
42-
${/*connect wire*/ is.statement}
48+
__establishContextfulRelationship(contextfulParent, instance);
49+
${/*connect wire*/ is.statement}
4350
44-
instance[__SYMBOL__SET_INTERNALS](
45-
props,
46-
attrs,
47-
__lwcPublicProperties__,
48-
__lwcPrivateProperties__,
49-
);
50-
instance.isConnected = true;
51-
if (instance.connectedCallback) {
52-
__mutationTracker.enable(instance);
53-
instance.connectedCallback();
54-
__mutationTracker.disable(instance);
55-
}
56-
// If a render() function is defined on the class or any of its superclasses, then that takes priority.
57-
// Next, if the class or any of its superclasses has an implicitly-associated template, then that takes
58-
// second priority (e.g. a foo.html file alongside a foo.js file). Finally, there is a fallback empty template.
59-
const tmplFn = instance.render?.() ?? ${/*component class*/ 0}[__SYMBOL__DEFAULT_TEMPLATE] ?? __fallbackTmpl;
60-
yield \`<\${tagName}\`;
51+
instance[__SYMBOL__SET_INTERNALS](
52+
props,
53+
attrs,
54+
__lwcPublicProperties__,
55+
__lwcPrivateProperties__,
56+
);
57+
instance.isConnected = true;
58+
if (instance.connectedCallback) {
59+
__mutationTracker.enable(instance);
60+
instance.connectedCallback();
61+
__mutationTracker.disable(instance);
62+
}
63+
// If a render() function is defined on the class or any of its superclasses, then that takes priority.
64+
// Next, if the class or any of its superclasses has an implicitly-associated template, then that takes
65+
// second priority (e.g. a foo.html file alongside a foo.js file). Finally, there is a fallback empty template.
66+
const tmplFn = instance.render?.() ?? ${/*component class*/ 0}[__SYMBOL__DEFAULT_TEMPLATE] ?? __fallbackTmpl;
67+
yield \`<\${tagName}\`;
6168
62-
const hostHasScopedStylesheets =
63-
tmplFn.hasScopedStylesheets ||
64-
hasScopedStaticStylesheets(${/*component class*/ 0});
65-
const hostScopeToken = hostHasScopedStylesheets ? tmplFn.stylesheetScopeToken + "-host" : undefined;
69+
const hostHasScopedStylesheets =
70+
tmplFn.hasScopedStylesheets ||
71+
hasScopedStaticStylesheets(${/*component class*/ 0});
72+
const hostScopeToken = hostHasScopedStylesheets ? tmplFn.stylesheetScopeToken + "-host" : undefined;
6673
67-
yield* __renderAttrs(instance, attrs, hostScopeToken, scopeToken);
68-
yield '>';
69-
yield* tmplFn(
70-
shadowSlottedContent,
71-
lightSlottedContent,
72-
scopedSlottedContent,
73-
${/*component class*/ 0},
74-
instance
75-
);
76-
yield \`</\${tagName}>\`;
77-
}
78-
${/* component class */ 0}.__lwcPublicProperties__ = __lwcPublicProperties__;
74+
yield* __renderAttrs(instance, attrs, hostScopeToken, scopeToken);
75+
yield '>';
76+
yield* tmplFn(
77+
shadowSlottedContent,
78+
lightSlottedContent,
79+
scopedSlottedContent,
80+
${/*component class*/ 0},
81+
instance
82+
);
83+
yield \`</\${tagName}>\`;
84+
}
85+
});
86+
Object.defineProperty(
87+
${/* component class */ 0},
88+
'__lwcPublicProperties__',
89+
{
90+
configurable: false,
91+
enumerable: false,
92+
writable: false,
93+
value: __lwcPublicProperties__
94+
}
95+
);
7996
`<[Statement]>;
8097

8198
const bExposeTemplate = esTemplate`
8299
if (${/*template*/ is.identifier}) {
83-
${/* component class */ is.identifier}[__SYMBOL__DEFAULT_TEMPLATE] = ${/*template*/ 0}
100+
Object.defineProperty(
101+
${/* component class */ is.identifier},
102+
__SYMBOL__DEFAULT_TEMPLATE,
103+
{
104+
configurable: false,
105+
enumerable: false,
106+
writable: false,
107+
value: ${/*template*/ 0}
108+
}
109+
);
84110
}
85111
`<IfStatement>;
86112

0 commit comments

Comments
 (0)