From 7a0fccfb71bb0a8d8ea52fb96a0ad81ff148a273 Mon Sep 17 00:00:00 2001 From: John Hefferman Date: Wed, 17 Sep 2025 12:12:31 -0600 Subject: [PATCH 1/3] fix: signals and tests --- .../src/framework/mutation-tracker.ts | 21 ++- packages/@lwc/features/src/index.ts | 1 + packages/@lwc/features/src/types.ts | 7 + .../test/signal/untrusted/index.spec.js | 126 ++++++++++++++++++ .../test/signal/untrusted/x/signal/signal.js | 39 ++++++ .../test/signal/untrusted/x/test/test.html | 3 + .../test/signal/untrusted/x/test/test.js | 5 + .../@lwc/shared/src/__tests__/signals.spec.ts | 10 +- packages/@lwc/shared/src/signals.ts | 17 ++- scripts/bundlesize/bundlesize.config.json | 2 +- 10 files changed, 223 insertions(+), 8 deletions(-) create mode 100644 packages/@lwc/integration-not-karma/test/signal/untrusted/index.spec.js create mode 100644 packages/@lwc/integration-not-karma/test/signal/untrusted/x/signal/signal.js create mode 100644 packages/@lwc/integration-not-karma/test/signal/untrusted/x/test/test.html create mode 100644 packages/@lwc/integration-not-karma/test/signal/untrusted/x/test/test.js diff --git a/packages/@lwc/engine-core/src/framework/mutation-tracker.ts b/packages/@lwc/engine-core/src/framework/mutation-tracker.ts index 3ed830fc64..d28bd0225e 100644 --- a/packages/@lwc/engine-core/src/framework/mutation-tracker.ts +++ b/packages/@lwc/engine-core/src/framework/mutation-tracker.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: MIT * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -import { isNull, isObject, isTrustedSignal } from '@lwc/shared'; +import { isNull, isObject, isTrustedSignal, legacyIsTrustedSignal } from '@lwc/shared'; import { ReactiveObserver, valueMutated, valueObserved } from '../libs/mutation-tracker'; import { subscribeToSignal } from '../libs/signal-tracker'; import type { Signal } from '@lwc/signals'; @@ -41,13 +41,26 @@ export function componentValueObserved(vm: VM, key: PropertyKey, target: any = { lwcRuntimeFlags.ENABLE_EXPERIMENTAL_SIGNALS && isObject(target) && !isNull(target) && - isTrustedSignal(target) && process.env.IS_BROWSER && // Only subscribe if a template is being rendered by the engine tro.isObserving() ) { - // Subscribe the template reactive observer's notify method, which will mark the vm as dirty and schedule hydration. - subscribeToSignal(component, target as Signal, tro.notify.bind(tro)); + /** + * The legacy validation behavior was that this check should only + * be performed for runtimes that have provided a trustedSignals set. + * However, this resulted in a bug as all object values were + * being considered signals in environments where the trustedSignals + * set had not been defined. The runtime flag has been added as a killswitch + * in case the fix needs to be reverted. + */ + if ( + lwcRuntimeFlags.ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION + ? legacyIsTrustedSignal(target) + : isTrustedSignal(target) + ) { + // Subscribe the template reactive observer's notify method, which will mark the vm as dirty and schedule hydration. + subscribeToSignal(component, target as Signal, tro.notify.bind(tro)); + } } } diff --git a/packages/@lwc/features/src/index.ts b/packages/@lwc/features/src/index.ts index 59dc9624f2..a60f471c9d 100644 --- a/packages/@lwc/features/src/index.ts +++ b/packages/@lwc/features/src/index.ts @@ -19,6 +19,7 @@ const features: FeatureFlagMap = { ENABLE_LEGACY_SCOPE_TOKENS: null, ENABLE_FORCE_SHADOW_MIGRATE_MODE: null, ENABLE_EXPERIMENTAL_SIGNALS: null, + ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION: null, DISABLE_SYNTHETIC_SHADOW: null, DISABLE_SCOPE_TOKEN_VALIDATION: null, LEGACY_LOCKER_ENABLED: null, diff --git a/packages/@lwc/features/src/types.ts b/packages/@lwc/features/src/types.ts index f66032f010..c15f2faf28 100644 --- a/packages/@lwc/features/src/types.ts +++ b/packages/@lwc/features/src/types.ts @@ -70,6 +70,13 @@ export interface FeatureFlagMap { */ ENABLE_EXPERIMENTAL_SIGNALS: FeatureFlagValue; + /** + * If true, legacy signal validation is used, where all component properties are considered signals or context + * if a trustedSignalSet and trustedContextSet have not been provided via setTrustedSignalSet and setTrustedContextSet. + * This is a killswitch for a bug fix: #5492 + */ + ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION: FeatureFlagValue; + /** * If true, ignore `@lwc/synthetic-shadow` even if it's loaded on the page. Instead, run all components in * native shadow mode. diff --git a/packages/@lwc/integration-not-karma/test/signal/untrusted/index.spec.js b/packages/@lwc/integration-not-karma/test/signal/untrusted/index.spec.js new file mode 100644 index 0000000000..f7c4ee9aaf --- /dev/null +++ b/packages/@lwc/integration-not-karma/test/signal/untrusted/index.spec.js @@ -0,0 +1,126 @@ +import { createElement, setFeatureFlagForTest } from 'lwc'; + +import Test from 'x/test'; +import { Signal } from 'x/signal'; +import { spyConsole } from '../../../helpers/console.js'; + +const createElementSignalAndInsertIntoDom = async (object) => { + const elm = createElement('x-test', { is: Test }); + elm.object = object; + document.body.appendChild(elm); + await Promise.resolve(); + return elm; +}; + +describe('signal reaction in lwc', () => { + let consoleSpy; + + beforeAll(() => setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', true)); + afterAll(() => setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', false)); + beforeEach(() => (consoleSpy = spyConsole())); + afterEach(() => consoleSpy.reset()); + + describe('with trusted signal set', () => { + describe('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is enabled', () => { + beforeAll(() => setFeatureFlagForTest('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', true)); + afterAll(() => setFeatureFlagForTest('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false)); + it('will not warn if rendering non-signal objects ', async () => { + const elm = await createElementSignalAndInsertIntoDom({ + value: 'non signal value', + }); + expect(consoleSpy.calls.warn.length).toEqual(0); + expect(elm.shadowRoot.textContent).toBe('non signal value'); + }); + it('will not warn if rendering signal objects', async () => { + const signal = new Signal('signal value'); + const elm = await createElementSignalAndInsertIntoDom(signal); + expect(consoleSpy.calls.warn.length).toEqual(0); + signal.value = 'new signal value'; + await Promise.resolve(); + expect(elm.shadowRoot.textContent).toBe('new signal value'); + }); + }); + + describe('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is disabled', () => { + beforeAll(() => + setFeatureFlagForTest('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false) + ); + it('will not warn if rendering non-signal objects', async () => { + const elm = await createElementSignalAndInsertIntoDom({ + value: 'non signal value', + }); + expect(consoleSpy.calls.warn.length).toEqual(0); + expect(elm.shadowRoot.textContent).toBe('non signal value'); + }); + it('will not warn if rendering signal objects', async () => { + const signal = new Signal('signal value'); + const elm = await createElementSignalAndInsertIntoDom(signal); + expect(consoleSpy.calls.warn.length).toEqual(0); + signal.value = 'new signal value'; + await Promise.resolve(); + expect(elm.shadowRoot.textContent).toBe('new signal value'); + }); + }); + }); + + describe('without trusted signal set', () => { + beforeAll(() => globalThis.__lwcResetTrustedSignalsSetForTest()); + describe('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is enabled', () => { + beforeAll(() => setFeatureFlagForTest('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', true)); + afterAll(() => setFeatureFlagForTest('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false)); + /** + * The legacy validation behavior was that this check should only + * be performed for runtimes that have provided a trustedSignals set. + * However, this resulted in a bug as all object values were + * being considered signals in environments where the trustedSignals + * set had not been defined. The runtime flag has been added as a killswitch + * in case the fix needs to be reverted. + */ + it('will warn if rendering non-signal objects ', async () => { + const elm = await createElementSignalAndInsertIntoDom({ + value: 'non signal value', + }); + expect(consoleSpy.calls.warn[0][0].message).toContain( + 'Attempted to subscribe to an object that has the shape of a signal but received the following error: TypeError: signal.subscribe is not a function' + ); + expect(elm.shadowRoot.textContent).toBe('non signal value'); + }); + it('will not warn if rendering signal objects', async () => { + const signal = new Signal('signal value'); + const elm = await createElementSignalAndInsertIntoDom(signal); + expect(consoleSpy.calls.warn.length).toEqual(0); + signal.value = 'new signal value'; + await Promise.resolve(); + expect(elm.shadowRoot.textContent).toBe('new signal value'); + }); + }); + + describe('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is disabled', () => { + beforeAll(() => + setFeatureFlagForTest('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false) + ); + it('will not warn if rendering non-signal objects', async () => { + const elm = await createElementSignalAndInsertIntoDom({ + value: 'non signal value', + }); + expect(consoleSpy.calls.warn.length).toEqual(0); + expect(elm.shadowRoot.textContent).toBe('non signal value'); + }); + /** + * Signals are designed to be used where trustedSignalSet has been defined via setTrustedSignalSet + * This is because checking against the set is an efficient way to determine if an object is a Signal + * This is acceptable as Signals is an internal API and designed to work where setTrustedSignalSet has been used. + * Because of this, the signal value does not change here. + * See #5347 for context. + */ + it('will not warn if rendering signal objects but it will not react', async () => { + const signal = new Signal('signal value'); + const elm = await createElementSignalAndInsertIntoDom(signal); + expect(consoleSpy.calls.warn.length).toEqual(0); + signal.value = 'new signal value'; + await Promise.resolve(); + expect(elm.shadowRoot.textContent).toBe('signal value'); + }); + }); + }); +}); diff --git a/packages/@lwc/integration-not-karma/test/signal/untrusted/x/signal/signal.js b/packages/@lwc/integration-not-karma/test/signal/untrusted/x/signal/signal.js new file mode 100644 index 0000000000..d56372c37c --- /dev/null +++ b/packages/@lwc/integration-not-karma/test/signal/untrusted/x/signal/signal.js @@ -0,0 +1,39 @@ +// 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. + +import { addTrustedSignal } from '../../../../../helpers/signals.js'; + +export class Signal { + subscribers = new Set(); + + constructor(initialValue) { + this._value = initialValue; + addTrustedSignal(this); + } + + set value(newValue) { + this._value = newValue; + this.notify(); + } + + get value() { + return this._value; + } + + subscribe(onUpdate) { + this.subscribers.add(onUpdate); + return () => { + this.subscribers.delete(onUpdate); + }; + } + + notify() { + for (const subscriber of this.subscribers) { + subscriber(); + } + } + + getSubscriberCount() { + return this.subscribers.size; + } +} diff --git a/packages/@lwc/integration-not-karma/test/signal/untrusted/x/test/test.html b/packages/@lwc/integration-not-karma/test/signal/untrusted/x/test/test.html new file mode 100644 index 0000000000..0dca7be2cf --- /dev/null +++ b/packages/@lwc/integration-not-karma/test/signal/untrusted/x/test/test.html @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/@lwc/integration-not-karma/test/signal/untrusted/x/test/test.js b/packages/@lwc/integration-not-karma/test/signal/untrusted/x/test/test.js new file mode 100644 index 0000000000..cff13b0901 --- /dev/null +++ b/packages/@lwc/integration-not-karma/test/signal/untrusted/x/test/test.js @@ -0,0 +1,5 @@ +import { LightningElement, api } from 'lwc'; + +export default class Test extends LightningElement { + @api object; +} diff --git a/packages/@lwc/shared/src/__tests__/signals.spec.ts b/packages/@lwc/shared/src/__tests__/signals.spec.ts index b958052a79..81f5243d1f 100644 --- a/packages/@lwc/shared/src/__tests__/signals.spec.ts +++ b/packages/@lwc/shared/src/__tests__/signals.spec.ts @@ -10,6 +10,7 @@ describe('signals', () => { let setTrustedSignalSet: (signals: WeakSet) => void; let addTrustedSignal: (signal: object) => void; let isTrustedSignal: (target: object) => boolean; + let legacyIsTrustedSignal: (target: object) => boolean; beforeEach(async () => { vi.resetModules(); @@ -17,6 +18,7 @@ describe('signals', () => { setTrustedSignalSet = signalsModule.setTrustedSignalSet; addTrustedSignal = signalsModule.addTrustedSignal; isTrustedSignal = signalsModule.isTrustedSignal; + legacyIsTrustedSignal = signalsModule.legacyIsTrustedSignal; }); describe('setTrustedSignalSet', () => { @@ -53,8 +55,12 @@ describe('signals', () => { expect(isTrustedSignal({})).toBe(false); }); - it('should return true for all calls when trustedSignals is not set', () => { - expect(isTrustedSignal({})).toBe(true); + it('should return false for all calls when trustedSignals is not set', () => { + expect(isTrustedSignal({})).toBe(false); + }); + + it('legacyIsTrustedSignal should return true when trustedSignals is not set', () => { + expect(legacyIsTrustedSignal({})).toBe(true); }); }); }); diff --git a/packages/@lwc/shared/src/signals.ts b/packages/@lwc/shared/src/signals.ts index 5bfb66b27c..9a5cccd3e4 100644 --- a/packages/@lwc/shared/src/signals.ts +++ b/packages/@lwc/shared/src/signals.ts @@ -19,7 +19,15 @@ export function addTrustedSignal(signal: object) { trustedSignals?.add(signal); } -export function isTrustedSignal(target: object): boolean { +/** + * The legacy validation behavior was that this check should only + * be performed for runtimes that have provided a trustedSignals set. + * However, this resulted in a bug as all object values were + * being considered signals in environments where the trustedSignals + * set had not been defined. The runtime flag has been added as a killswitch + * in case the fix needs to be reverted. + */ +export function legacyIsTrustedSignal(target: object): boolean { if (!trustedSignals) { // The runtime didn't set a trustedSignals set // this check should only be performed for runtimes that care about filtering signals to track @@ -28,3 +36,10 @@ export function isTrustedSignal(target: object): boolean { } return trustedSignals.has(target); } + +export function isTrustedSignal(target: object): boolean { + if (!trustedSignals) { + return false; + } + return trustedSignals.has(target); +} diff --git a/scripts/bundlesize/bundlesize.config.json b/scripts/bundlesize/bundlesize.config.json index a4fa1b488c..624cd12230 100644 --- a/scripts/bundlesize/bundlesize.config.json +++ b/scripts/bundlesize/bundlesize.config.json @@ -2,7 +2,7 @@ "files": [ { "path": "packages/@lwc/engine-dom/dist/index.js", - "maxSize": "25.0KB" + "maxSize": "24.73KB" }, { "path": "packages/@lwc/synthetic-shadow/dist/index.js", From 3b61b5e6a5dfff269e532546e5d5874051e102e6 Mon Sep 17 00:00:00 2001 From: John Hefferman Date: Wed, 17 Sep 2025 12:15:56 -0600 Subject: [PATCH 2/3] fix: add global test fn for signals --- packages/@lwc/shared/src/signals.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/@lwc/shared/src/signals.ts b/packages/@lwc/shared/src/signals.ts index 9a5cccd3e4..1de7f22fd7 100644 --- a/packages/@lwc/shared/src/signals.ts +++ b/packages/@lwc/shared/src/signals.ts @@ -6,12 +6,19 @@ */ import { isFalse } from './assert'; -let trustedSignals: WeakSet; +let trustedSignals: WeakSet | undefined; export function setTrustedSignalSet(signals: WeakSet) { isFalse(trustedSignals, 'Trusted Signal Set is already set!'); trustedSignals = signals; + + // Only used in LWC's Karma. Contained within the set function as there are multiple imports of + // this module. Placing it here ensures we reference the import where the trustedSignals set is maintained + if (process.env.NODE_ENV === 'test-karma-lwc') { + // Used to reset the global state between test runs + (globalThis as any).__lwcResetTrustedSignalsSetForTest = () => (trustedSignals = undefined); + } } export function addTrustedSignal(signal: object) { From d167f6fbb5bbf27a2fe49e311eb34e2af0e09a66 Mon Sep 17 00:00:00 2001 From: John Hefferman Date: Wed, 17 Sep 2025 12:44:23 -0600 Subject: [PATCH 3/3] chore: because will --- scripts/bundlesize/bundlesize.config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/bundlesize/bundlesize.config.json b/scripts/bundlesize/bundlesize.config.json index 624cd12230..57c8efea07 100644 --- a/scripts/bundlesize/bundlesize.config.json +++ b/scripts/bundlesize/bundlesize.config.json @@ -2,7 +2,7 @@ "files": [ { "path": "packages/@lwc/engine-dom/dist/index.js", - "maxSize": "24.73KB" + "maxSize": "24.72KB" }, { "path": "packages/@lwc/synthetic-shadow/dist/index.js",