From a310e55bca9966560b83814470a69f78eae7ff8c Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Wed, 8 Jan 2025 09:28:46 -0800 Subject: [PATCH 1/5] fix(signals): avoid throwing when `in` check throws Fixes #5104 --- .../src/framework/mutation-tracker.ts | 9 ++++---- .../@lwc/engine-core/src/framework/utils.ts | 10 ++++++++ .../test/signal/protocol/index.spec.js | 10 ++++++++ .../test/signal/protocol/x/throws/throws.html | 3 +++ .../test/signal/protocol/x/throws/throws.js | 23 +++++++++++++++++++ 5 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 packages/@lwc/integration-karma/test/signal/protocol/x/throws/throws.html create mode 100644 packages/@lwc/integration-karma/test/signal/protocol/x/throws/throws.js diff --git a/packages/@lwc/engine-core/src/framework/mutation-tracker.ts b/packages/@lwc/engine-core/src/framework/mutation-tracker.ts index 33011bb19c..8e45e3b584 100644 --- a/packages/@lwc/engine-core/src/framework/mutation-tracker.ts +++ b/packages/@lwc/engine-core/src/framework/mutation-tracker.ts @@ -7,6 +7,7 @@ import { isFunction, isNull, isObject, isTrustedSignal } from '@lwc/shared'; import { ReactiveObserver, valueMutated, valueObserved } from '../libs/mutation-tracker'; import { subscribeToSignal } from '../libs/signal-tracker'; +import { safeHasProp } from './utils'; import type { Signal } from '@lwc/signals'; import type { JobFunction, CallbackFunction } from '../libs/mutation-tracker'; import type { VM } from './vm'; @@ -34,16 +35,16 @@ export function componentValueObserved(vm: VM, key: PropertyKey, target: any = { } // The portion of reactivity that's exposed to signals is to subscribe a callback to re-render the VM (templates). - // We check check the following to ensure re-render is subscribed at the correct time. + // We check the following to ensure re-render is subscribed at the correct time. // 1. The template is currently being rendered (there is a template reactive observer) // 2. There was a call to a getter to access the signal (happens during vnode generation) if ( lwcRuntimeFlags.ENABLE_EXPERIMENTAL_SIGNALS && isObject(target) && !isNull(target) && - 'value' in target && - 'subscribe' in target && - isFunction(target.subscribe) && + safeHasProp(target, 'value') && + safeHasProp(target, 'subscribe') && + isFunction((target as any).subscribe) && isTrustedSignal(target) && // Only subscribe if a template is being rendered by the engine tro.isObserving() diff --git a/packages/@lwc/engine-core/src/framework/utils.ts b/packages/@lwc/engine-core/src/framework/utils.ts index 2a5163d3a5..1d0a03de9b 100644 --- a/packages/@lwc/engine-core/src/framework/utils.ts +++ b/packages/@lwc/engine-core/src/framework/utils.ts @@ -104,3 +104,13 @@ export function shouldBeFormAssociated(Ctor: LightningElementConstructor) { return ctorFormAssociated && apiFeatureEnabled; } + +// check if a property is in an object, and if the object throws an error merely because we are +// checking if the property exists, return false +export function safeHasProp(obj: any, prop: string) { + try { + return prop in obj; + } catch (_err) { + return false; + } +} diff --git a/packages/@lwc/integration-karma/test/signal/protocol/index.spec.js b/packages/@lwc/integration-karma/test/signal/protocol/index.spec.js index 4284cb59d9..4908057c55 100644 --- a/packages/@lwc/integration-karma/test/signal/protocol/index.spec.js +++ b/packages/@lwc/integration-karma/test/signal/protocol/index.spec.js @@ -6,6 +6,7 @@ import Parent from 'x/parent'; import Child from 'x/child'; import DuplicateSignalOnTemplate from 'x/duplicateSignalOnTemplate'; import List from 'x/list'; +import Throws from 'x/throws'; // Note for testing purposes the signal implementation uses LWC module resolution to simplify things. // In production the signal will come from a 3rd party library. @@ -212,6 +213,15 @@ describe('signal protocol', () => { expect(subscribe).not.toHaveBeenCalled(); }); + + fit('does not throw an error for objects that throw upon "in" checks', async () => { + const elm = createElement('x-throws', { is: Throws }); + document.body.appendChild(elm); + + await Promise.resolve(); + + expect(elm.shadowRoot.querySelector('h1').textContent).toBe('hello'); + }); }); describe('ENABLE_EXPERIMENTAL_SIGNALS not set', () => { diff --git a/packages/@lwc/integration-karma/test/signal/protocol/x/throws/throws.html b/packages/@lwc/integration-karma/test/signal/protocol/x/throws/throws.html new file mode 100644 index 0000000000..e40c3bd251 --- /dev/null +++ b/packages/@lwc/integration-karma/test/signal/protocol/x/throws/throws.html @@ -0,0 +1,3 @@ + diff --git a/packages/@lwc/integration-karma/test/signal/protocol/x/throws/throws.js b/packages/@lwc/integration-karma/test/signal/protocol/x/throws/throws.js new file mode 100644 index 0000000000..21f0811441 --- /dev/null +++ b/packages/@lwc/integration-karma/test/signal/protocol/x/throws/throws.js @@ -0,0 +1,23 @@ +import { LightningElement } from 'lwc'; + +export default class extends LightningElement { + foo; + + constructor() { + super(); + + this.foo = new Proxy( + {}, + { + has() { + throw new Error("oh no you don't!"); + }, + } + ); + } + + renderedCallback() { + // access `this.foo` to trigger mutation-tracker.ts + this.bar = this.foo; + } +} From 175aac796c40e20090f046c79e4301ecdf8793f9 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Wed, 8 Jan 2025 09:30:39 -0800 Subject: [PATCH 2/5] test: remove fit() --- .../@lwc/integration-karma/test/signal/protocol/index.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@lwc/integration-karma/test/signal/protocol/index.spec.js b/packages/@lwc/integration-karma/test/signal/protocol/index.spec.js index 4908057c55..775661f5a6 100644 --- a/packages/@lwc/integration-karma/test/signal/protocol/index.spec.js +++ b/packages/@lwc/integration-karma/test/signal/protocol/index.spec.js @@ -214,7 +214,7 @@ describe('signal protocol', () => { expect(subscribe).not.toHaveBeenCalled(); }); - fit('does not throw an error for objects that throw upon "in" checks', async () => { + it('does not throw an error for objects that throw upon "in" checks', async () => { const elm = createElement('x-throws', { is: Throws }); document.body.appendChild(elm); From 1fad66ee199eb0fc131945c0457890fd63503c47 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Wed, 8 Jan 2025 09:38:27 -0800 Subject: [PATCH 3/5] Update packages/@lwc/engine-core/src/framework/utils.ts Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com> --- packages/@lwc/engine-core/src/framework/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@lwc/engine-core/src/framework/utils.ts b/packages/@lwc/engine-core/src/framework/utils.ts index 1d0a03de9b..7effb4b0e3 100644 --- a/packages/@lwc/engine-core/src/framework/utils.ts +++ b/packages/@lwc/engine-core/src/framework/utils.ts @@ -107,7 +107,7 @@ export function shouldBeFormAssociated(Ctor: LightningElementConstructor) { // check if a property is in an object, and if the object throws an error merely because we are // checking if the property exists, return false -export function safeHasProp(obj: any, prop: string) { +export function safeHasProp(obj: T, prop: K): obj is Record { try { return prop in obj; } catch (_err) { From a3d0b0fbe061faf05a09911ec4a3819cc89e069e Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Wed, 8 Jan 2025 09:38:32 -0800 Subject: [PATCH 4/5] Update packages/@lwc/engine-core/src/framework/mutation-tracker.ts Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com> --- packages/@lwc/engine-core/src/framework/mutation-tracker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@lwc/engine-core/src/framework/mutation-tracker.ts b/packages/@lwc/engine-core/src/framework/mutation-tracker.ts index 8e45e3b584..d179ea9d88 100644 --- a/packages/@lwc/engine-core/src/framework/mutation-tracker.ts +++ b/packages/@lwc/engine-core/src/framework/mutation-tracker.ts @@ -44,7 +44,7 @@ export function componentValueObserved(vm: VM, key: PropertyKey, target: any = { !isNull(target) && safeHasProp(target, 'value') && safeHasProp(target, 'subscribe') && - isFunction((target as any).subscribe) && + isFunction(target.subscribe) && isTrustedSignal(target) && // Only subscribe if a template is being rendered by the engine tro.isObserving() From 5edfa007fe5de26b574674c048707b9618d5e433 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Wed, 8 Jan 2025 09:57:51 -0800 Subject: [PATCH 5/5] fix: make typescript shut up --- packages/@lwc/engine-core/src/framework/utils.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/@lwc/engine-core/src/framework/utils.ts b/packages/@lwc/engine-core/src/framework/utils.ts index 7effb4b0e3..2bc586e4fd 100644 --- a/packages/@lwc/engine-core/src/framework/utils.ts +++ b/packages/@lwc/engine-core/src/framework/utils.ts @@ -107,9 +107,12 @@ export function shouldBeFormAssociated(Ctor: LightningElementConstructor) { // check if a property is in an object, and if the object throws an error merely because we are // checking if the property exists, return false -export function safeHasProp(obj: T, prop: K): obj is Record { +export function safeHasProp( + obj: unknown, + prop: K +): obj is Record { try { - return prop in obj; + return prop in (obj as any); } catch (_err) { return false; }