Signals validation fix backport

.nucleus.yaml

@@ -32,7 +32,7 @@ branches:
         pull-request:
             <<: *branch-definition
             merge-method: force-push # release branch should always be in sync with master branch (linear history)
-    winter25:
+    winter26:
         pull-request:
             <<: *branch-definition
     spring25:

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<unknown>, 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<unknown>, tro.notify.bind(tro));
+        }
     }
 }
 

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,

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.

packages/@lwc/integration-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 'test-utils';
+
+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');
+            });
+        });
+    });
+});

packages/@lwc/integration-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 'test-utils';
+
+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;
+    }
+}

packages/@lwc/integration-karma/test/signal/untrusted/x/test/test.html

@@ -0,0 +1,3 @@
+<template>
+    {object.value}
+</template>

packages/@lwc/integration-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;
+}

packages/@lwc/shared/src/__tests__/signals.spec.ts

@@ -10,13 +10,15 @@ describe('signals', () => {
     let setTrustedSignalSet: (signals: WeakSet<object>) => void;
     let addTrustedSignal: (signal: object) => void;
     let isTrustedSignal: (target: object) => boolean;
+    let legacyIsTrustedSignal: (target: object) => boolean;
 
     beforeEach(async () => {
         vi.resetModules();
         const signalsModule = await import('../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);
         });
     });
 });

packages/@lwc/shared/src/signals.ts

@@ -6,20 +6,35 @@
  */
 import { isFalse } from './assert';
 
-let trustedSignals: WeakSet<object>;
+let trustedSignals: WeakSet<object> | undefined;
 
 export function setTrustedSignalSet(signals: WeakSet<object>) {
     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) {
     // This should be a no-op when the trustedSignals set isn't set by runtime
     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 +43,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);
+}

scripts/bundlesize/bundlesize.config.json

@@ -2,7 +2,7 @@
     "files": [
         {
             "path": "packages/@lwc/engine-dom/dist/index.js",
-            "maxSize": "24.68KB"
+            "maxSize": "24.72KB"
         },
         {
             "path": "packages/@lwc/synthetic-shadow/dist/index.js",