Skip to content

Commit

Permalink
fix(ssr): throw compile-time error on @wire-decorated getter/setter/m…
Browse files Browse the repository at this point in the history
…ethod (#5175)

* fix(ssr): throw compile-time error on @wire-decorated getter/setter/method

* fix(ssr): disallow @wire on computed props
  • Loading branch information
wjhsf authored Jan 30, 2025
1 parent 1193957 commit 476bb41
Show file tree
Hide file tree
Showing 39 changed files with 169 additions and 16 deletions.
18 changes: 10 additions & 8 deletions packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { rollup } from 'rollup';
import lwcRollupPlugin from '@lwc/rollup-plugin';
import { testFixtureDir, formatHTML } from '@lwc/test-utils-lwc-internals';
import { setFeatureFlagForTest } from '../index';
import type { FeatureFlagName } from '@lwc/features/dist/types';
import type { RollupLwcOptions } from '@lwc/rollup-plugin';
import type * as lwc from '../index';

Expand Down Expand Up @@ -128,16 +129,17 @@ function testFixtures(options?: RollupLwcOptions) {
// the LightningElement. Therefor the compiled module should also be evaluated in the
// same sandbox registry as the engine.
const lwcEngineServer = await import('../index');
const module = (await import(compiledFixturePath)) as FixtureModule;

const features = module!.features ?? [];
features.forEach((flag) => {
lwcEngineServer!.setFeatureFlagForTest(flag, true);
});

let result;
let err;
let features: FeatureFlagName[] = [];
try {
const module = (await import(compiledFixturePath)) as FixtureModule;

features = module!.features ?? [];
features.forEach((flag) => {
lwcEngineServer!.setFeatureFlagForTest(flag, true);
});
result = formatHTML(
lwcEngineServer!.renderComponent(
module!.tagName,
Expand All @@ -146,10 +148,10 @@ function testFixtures(options?: RollupLwcOptions) {
)
);
} catch (_err: any) {
if (_err.name === 'AssertionError') {
if (_err?.name === 'AssertionError') {
throw _err;
}
err = _err.message;
err = _err?.message || 'An empty error occurred?!';
}

features.forEach((flag) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"ssrFiles": {
"error": "error-ssr.txt",
"expected": "expected-ssr.html"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Unknown state: property definition has no key
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<x-wire>
<template shadowrootmode="open">
<p>
Prop named symbol that should not be used: wire data
</p>
<p>
Prop with symbol key that should be used: unset
</p>
</template>
</x-wire>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export class adapter {
constructor(dataCallback) {
this.dc = dataCallback;
}

connect() {}

update(config) {
this.dc(config.value);
}

disconnect() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<template>
<p>Prop named symbol that should not be used: {symbol}</p>
<p>Prop with symbol key that should be used: {symbolValue}</p>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { LightningElement, wire } from 'lwc';
import { adapter } from './adapter';

const symbol = Symbol("I'm a symbol!");
export default class extends LightningElement {
symbol = 'accidentally overwritten';

@wire(adapter, { value: 'wire data' })
[symbol] = 'unset';

get symbolValue() {
return this[symbol] ?? 'unset';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"ssrFiles": {
"error": "error-ssr.txt",
"expected": "expected-ssr.html"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@wire cannot be used on computed properties in SSR context.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const tagName = 'x-wire';
export { default } from 'x/wire';
export * from 'x/wire';
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ export default class Wire extends LightningElement {
throw new Error('getter should not be called');
}

@wire(adapter, { name: 'symbol' })
set [sym](v) {
throw new Error('setter should not be called');
}

get exposedSymbol() {
return this[sym];
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"ssrFiles": {
"error": "error-ssr.txt",
"expected": "expected-ssr.html"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[7:13]: Unexpected token: '{'
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
An empty error occurred?!
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const tagName = 'x-wire';
export { default } from 'x/wire';
export * from 'x/wire';
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export class adapter {
constructor(dataCallback) {
this.dc = dataCallback;
}

connect() {}

update(config) {
this.dc(config.name);
}

disconnect() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
{exposedSymbol}
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { LightningElement, wire } from 'lwc';
import { adapter } from './adapter';
const sym = Symbol('computed prop');

export default class Wire extends LightningElement {
@wire(adapter, { name: 'symbol' })
[sym]() {
return 'john was here';
}

get exposedSymbol() {
return this[sym]();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"ssrFiles": {
"error": "error-ssr.txt",
"expected": "expected-ssr.html"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@wire cannot be used on computed properties in SSR context.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
setter should not be called
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const tagName = 'x-wire';
export { default } from 'x/wire';
export * from 'x/wire';
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export class adapter {
constructor(dataCallback) {
this.dc = dataCallback;
}

connect() {}

update(config) {
this.dc(config.name);
}

disconnect() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
{exposedSymbol}
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { LightningElement, wire } from 'lwc';
import { adapter } from './adapter';
const sym = Symbol('computed prop');

export default class Wire extends LightningElement {
@wire(adapter, { name: 'symbol' })
set [sym](_) {
throw new Error('setter should not be called');
}

get exposedSymbol() {
this[sym] = 123;
return 123;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@ export const expectedFailures = new Set([
'attribute-global-html/as-component-prop/without-@api/index.js',
'exports/component-as-default/index.js',
'known-boolean-attributes/default-def-html-attributes/static-on-component/index.js',
'wire/errors/throws-on-computed-key/index.js',
'wire/errors/throws-when-colliding-prop-then-method/index.js',
]);
20 changes: 18 additions & 2 deletions packages/@lwc/ssr-compiler/src/compile-js/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ const visitors: Visitors = {
},
PropertyDefinition(path, state) {
const node = path.node;
if (!is.identifier(node?.key)) {
if (!node?.key) {
// Seems to occur for `@wire() [symbol];` -- not sure why
throw new Error('Unknown state: property definition has no key');
}
if (!is.identifier(node.key)) {
return;
}

Expand All @@ -107,6 +111,10 @@ const visitors: Visitors = {
is.identifier(decoratedExpression.callee) &&
decoratedExpression.callee.name === 'wire'
) {
if (node.computed) {
// TODO [#5032]: Harmonize errors thrown in `@lwc/ssr-compiler`
throw new Error('@wire cannot be used on computed properties in SSR context.');
}
catalogWireAdapters(path, state);
state.privateFields.push(node.key.name);
} else {
Expand Down Expand Up @@ -146,9 +154,17 @@ const visitors: Visitors = {
is.identifier(decoratedExpression.callee) &&
decoratedExpression.callee.name === 'wire'
) {
// not a getter/setter
const isRealMethod = node.kind === 'method';
if (node.computed) {
// TODO [#5032]: Harmonize errors thrown in `@lwc/ssr-compiler`
throw new Error(
`@wire cannot be used on computed ${isRealMethod ? 'method' : 'properties'} in SSR context.`
);
}
// Getters and setters are methods in the AST, but treated as properties by @wire
// Note that this means that their implementations are ignored!
if (node.kind === 'get' || node.kind === 'set') {
if (!isRealMethod) {
const methodAsProp = b.propertyDefinition(
structuredClone(node.key),
null,
Expand Down

0 comments on commit 476bb41

Please sign in to comment.