Skip to content

Commit a2c74f2

Browse files
nolanlawsonwjhsf
andauthored
fix(signals): avoid throwing when in check throws [backport] (#5110)
Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>
1 parent 9fed79b commit a2c74f2

File tree

5 files changed

+53
-3
lines changed

5 files changed

+53
-3
lines changed

packages/@lwc/engine-core/src/framework/mutation-tracker.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import { isFunction, isNull, isObject, isTrustedSignal } from '@lwc/shared';
88
import { ReactiveObserver, valueMutated, valueObserved } from '../libs/mutation-tracker';
99
import { subscribeToSignal } from '../libs/signal-tracker';
10+
import { safeHasProp } from './utils';
1011
import type { Signal } from '@lwc/signals';
1112
import type { JobFunction, CallbackFunction } from '../libs/mutation-tracker';
1213
import type { VM } from './vm';
@@ -34,15 +35,15 @@ export function componentValueObserved(vm: VM, key: PropertyKey, target: any = {
3435
}
3536

3637
// The portion of reactivity that's exposed to signals is to subscribe a callback to re-render the VM (templates).
37-
// We check check the following to ensure re-render is subscribed at the correct time.
38+
// We check the following to ensure re-render is subscribed at the correct time.
3839
// 1. The template is currently being rendered (there is a template reactive observer)
3940
// 2. There was a call to a getter to access the signal (happens during vnode generation)
4041
if (
4142
lwcRuntimeFlags.ENABLE_EXPERIMENTAL_SIGNALS &&
4243
isObject(target) &&
4344
!isNull(target) &&
44-
'value' in target &&
45-
'subscribe' in target &&
45+
safeHasProp(target, 'value') &&
46+
safeHasProp(target, 'subscribe') &&
4647
isFunction(target.subscribe) &&
4748
isTrustedSignal(target) &&
4849
// Only subscribe if a template is being rendered by the engine

packages/@lwc/engine-core/src/framework/utils.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,16 @@ export function shouldBeFormAssociated(Ctor: LightningElementConstructor) {
104104

105105
return ctorFormAssociated && apiFeatureEnabled;
106106
}
107+
108+
// check if a property is in an object, and if the object throws an error merely because we are
109+
// checking if the property exists, return false
110+
export function safeHasProp<K extends PropertyKey>(
111+
obj: unknown,
112+
prop: K
113+
): obj is Record<K, unknown> {
114+
try {
115+
return prop in (obj as any);
116+
} catch (_err) {
117+
return false;
118+
}
119+
}

packages/@lwc/integration-karma/test/signal/protocol/index.spec.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import Parent from 'x/parent';
66
import Child from 'x/child';
77
import DuplicateSignalOnTemplate from 'x/duplicateSignalOnTemplate';
88
import List from 'x/list';
9+
import Throws from 'x/throws';
910

1011
// Note for testing purposes the signal implementation uses LWC module resolution to simplify things.
1112
// In production the signal will come from a 3rd party library.
@@ -212,6 +213,15 @@ describe('signal protocol', () => {
212213

213214
expect(subscribe).not.toHaveBeenCalled();
214215
});
216+
217+
it('does not throw an error for objects that throw upon "in" checks', async () => {
218+
const elm = createElement('x-throws', { is: Throws });
219+
document.body.appendChild(elm);
220+
221+
await Promise.resolve();
222+
223+
expect(elm.shadowRoot.querySelector('h1').textContent).toBe('hello');
224+
});
215225
});
216226

217227
describe('ENABLE_EXPERIMENTAL_SIGNALS not set', () => {
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<template>
2+
<h1>hello</h1>
3+
</template>
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { LightningElement } from 'lwc';
2+
3+
export default class extends LightningElement {
4+
foo;
5+
6+
constructor() {
7+
super();
8+
9+
this.foo = new Proxy(
10+
{},
11+
{
12+
has() {
13+
throw new Error("oh no you don't!");
14+
},
15+
}
16+
);
17+
}
18+
19+
renderedCallback() {
20+
// access `this.foo` to trigger mutation-tracker.ts
21+
this.bar = this.foo;
22+
}
23+
}

0 commit comments

Comments
 (0)