Replace isTrustedSignal with symbol

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

@@ -5,9 +5,9 @@
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
  */
 import { isNull, isObject, isTrustedSignal, legacyIsTrustedSignal } from '@lwc/shared';
+import { type Signal, isLwcSignal } from '@lwc/signals';
 import { ReactiveObserver, valueMutated, valueObserved } from '../libs/mutation-tracker';
 import { subscribeToSignal } from '../libs/signal-tracker';
-import type { Signal } from '@lwc/signals';
 import type { JobFunction, CallbackFunction } from '../libs/mutation-tracker';
 import type { VM } from './vm';
 
@@ -45,6 +45,9 @@ export function componentValueObserved(vm: VM, key: PropertyKey, target: any = {
         // Only subscribe if a template is being rendered by the engine
         tro.isObserving()
     ) {
+        // TODO [#123]: add gate
+        const lwcSignal = (target as any)[isLwcSignal];
+
         /**
          * The legacy validation behavior was that this check should only
          * be performed for runtimes that have provided a trustedSignals set.
@@ -53,11 +56,11 @@ export function componentValueObserved(vm: VM, key: PropertyKey, target: any = {
          * 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)
-        ) {
+        const trustedSignal = lwcRuntimeFlags.ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION
+            ? legacyIsTrustedSignal(target)
+            : isTrustedSignal(target);
+
+        if (lwcSignal || trustedSignal) {
             // 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/integration-not-karma/test/signal-lwc/protocol/x/signal/signal.js

@@ -0,0 +1,38 @@
+// 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 { SignalBaseClass } from 'lwc';
+
+export class Signal extends SignalBaseClass {
+    removedSubscribers = [];
+
+    constructor(initialValue) {
+        super();
+        this.value = initialValue;
+    }
+
+    set value(newValue) {
+        this._value = newValue;
+        this.notify();
+    }
+
+    get value() {
+        return this._value;
+    }
+
+    subscribe(onUpdate) {
+        this.subscribers.add(onUpdate);
+        return () => {
+            this.subscribers.delete(onUpdate);
+            this.removedSubscribers.push(onUpdate);
+        };
+    }
+
+    getSubscriberCount() {
+        return this.subscribers.size;
+    }
+
+    getRemovedSubscriberCount() {
+        return this.removedSubscribers.length;
+    }
+}

packages/@lwc/integration-not-karma/test/signal-lwc/reactivity/x/signal/signal.js

@@ -0,0 +1,24 @@
+// 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 { SignalBaseClass } from 'lwc';
+
+export class Signal extends SignalBaseClass {
+    constructor(initialValue) {
+        super();
+        this.value = initialValue;
+    }
+
+    set value(newValue) {
+        this._value = newValue;
+        this.notify();
+    }
+
+    get value() {
+        return this._value;
+    }
+
+    getSubscriberCount() {
+        return this.subscribers.size;
+    }
+}

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

@@ -0,0 +1,252 @@
+import { createElement, setFeatureFlagForTest } from 'lwc';
+import Reactive from 'x/reactive';
+import NonReactive from 'x/nonReactive';
+import Container from 'x/container';
+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.
+import { Signal } from 'x/signal';
+import { fn as mockFn } from '@vitest/spy';
+import { resetDOM } from '../../../helpers/reset.js';
+
+describe('signal protocol', () => {
+    beforeAll(() => {
+        setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', true);
+    });
+
+    afterAll(() => {
+        setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', false);
+    });
+
+    afterEach(resetDOM);
+
+    describe('lwc engine subscribes template re-render callback when signal is bound to an LWC and used on a template', () => {
+        [
+            {
+                testName: 'contains a getter that references a bound signal (.value on template)',
+                flag: 'showGetterSignal',
+            },
+            {
+                testName: 'contains a getter that references a bound signal value',
+                flag: 'showOnlyUsingSignalNotValue',
+            },
+            {
+                testName: 'contains a signal with @api annotation (.value on template)',
+                flag: 'showApiSignal',
+            },
+            {
+                testName: 'contains a signal with @track annotation (.value on template)',
+                flag: 'showTrackedSignal',
+            },
+            {
+                testName: 'contains an observed field referencing a signal (.value on template)',
+                flag: 'showObservedFieldSignal',
+            },
+            {
+                testName: 'contains a direct reference to a signal (not .value) in the template',
+                flag: 'showOnlyUsingSignalNotValue',
+            },
+        ].forEach(({ testName, flag }) => {
+            // Test all ways of binding signal to an LWC + template that cause re-rendering
+            it(testName, async () => {
+                const elm = createElement('x-reactive', { is: Reactive });
+                document.body.appendChild(elm);
+                await Promise.resolve();
+
+                expect(elm.getSignalSubscriberCount()).toBe(0);
+                elm[flag] = true;
+                await Promise.resolve();
+
+                // the engine will automatically subscribe the re-render callback
+                expect(elm.getSignalSubscriberCount()).toBe(1);
+            });
+        });
+    });
+
+    it('lwc engine should automatically unsubscribe the re-render callback if signal is not used on a template', async () => {
+        const elm = createElement('x-reactive', { is: Reactive });
+        elm.showObservedFieldSignal = true;
+        document.body.appendChild(elm);
+        await Promise.resolve();
+
+        expect(elm.getSignalSubscriberCount()).toBe(1);
+        elm.showObservedFieldSignal = false;
+        await Promise.resolve();
+
+        expect(elm.getSignalSubscriberCount()).toBe(0);
+        document.body.removeChild(elm);
+    });
+
+    it('lwc engine does not subscribe re-render callback if signal is not used on a template', async () => {
+        const elm = createElement('x-non-reactive', { is: NonReactive });
+        document.body.appendChild(elm);
+        await Promise.resolve();
+
+        expect(elm.getSignalSubscriberCount()).toBe(0);
+    });
+
+    it('only the components referencing a signal should re-render', async () => {
+        const container = createElement('x-container', { is: Container });
+        // append the container first to avoid error message with native lifecycle
+        document.body.appendChild(container);
+        await Promise.resolve();
+
+        const signalElm = createElement('x-signal-elm', { is: Child });
+        const signal = new Signal('initial value');
+        signalElm.signal = signal;
+        container.appendChild(signalElm);
+        await Promise.resolve();
+
+        expect(container.renderCount).toBe(1);
+        expect(signalElm.renderCount).toBe(1);
+        expect(signal.getSubscriberCount()).toBe(1);
+
+        signal.value = 'updated value';
+        await Promise.resolve();
+
+        expect(container.renderCount).toBe(1);
+        expect(signalElm.renderCount).toBe(2);
+        expect(signal.getSubscriberCount()).toBe(1);
+    });
+
+    it('only subscribes the re-render callback a single time when signal is referenced multiple times on a template', async () => {
+        const elm = createElement('x-duplicate-signals-on-template', {
+            is: DuplicateSignalOnTemplate,
+        });
+        document.body.appendChild(elm);
+        await Promise.resolve();
+
+        expect(elm.renderCount).toBe(1);
+        expect(elm.getSignalSubscriberCount()).toBe(1);
+        expect(elm.getSignalRemovedSubscriberCount()).toBe(0);
+
+        elm.updateSignalValue();
+        await Promise.resolve();
+
+        expect(elm.renderCount).toBe(2);
+        expect(elm.getSignalSubscriberCount()).toBe(1);
+        expect(elm.getSignalRemovedSubscriberCount()).toBe(1);
+    });
+
+    it('only subscribes re-render callback a single time when signal is referenced multiple times in a list', async () => {
+        const elm = createElement('x-list', { is: List });
+        const signal = new Signal('initial value');
+        elm.signal = signal;
+        document.body.appendChild(elm);
+        await Promise.resolve();
+
+        expect(signal.getSubscriberCount()).toBe(1);
+        expect(signal.getRemovedSubscriberCount()).toBe(0);
+
+        document.body.removeChild(elm);
+        await Promise.resolve();
+
+        expect(signal.getSubscriberCount()).toBe(0);
+        expect(signal.getRemovedSubscriberCount()).toBe(1);
+    });
+
+    it('unsubscribes when element is removed from the dom', async () => {
+        const elm = createElement('x-child', { is: Child });
+        const signal = new Signal('initial value');
+        elm.signal = signal;
+        document.body.appendChild(elm);
+        await Promise.resolve();
+
+        expect(signal.getSubscriberCount()).toBe(1);
+        expect(signal.getRemovedSubscriberCount()).toBe(0);
+
+        document.body.removeChild(elm);
+        await Promise.resolve();
+
+        expect(signal.getSubscriberCount()).toBe(0);
+        expect(signal.getRemovedSubscriberCount()).toBe(1);
+    });
+
+    it('on template re-render unsubscribes all components where signal is not present on the template', async () => {
+        const elm = createElement('x-parent', { is: Parent });
+        elm.showChild = true;
+
+        document.body.appendChild(elm);
+        await Promise.resolve();
+
+        // subscribed both parent and child
+        // as long as parent contains reference to the signal, even if it's just to pass it to a child
+        // it will be subscribed.
+        expect(elm.getSignalSubscriberCount()).toBe(2);
+        expect(elm.getSignalRemovedSubscriberCount()).toBe(0);
+
+        elm.showChild = false;
+        await Promise.resolve();
+
+        // The signal is not being used on the parent template anymore so it will be removed
+        expect(elm.getSignalSubscriberCount()).toBe(0);
+        expect(elm.getSignalRemovedSubscriberCount()).toBe(2);
+    });
+
+    it('does not subscribe if the signal shape is incorrect', async () => {
+        const elm = createElement('x-child', { is: Child });
+        const subscribe = mockFn();
+        // Note the signals property is value's' and not value
+        const signal = { values: 'initial value', subscribe };
+        elm.signal = signal;
+        document.body.appendChild(elm);
+        await Promise.resolve();
+
+        expect(subscribe).not.toHaveBeenCalled();
+    });
+
+    it('does not subscribe if the signal is not added as trusted signal', async () => {
+        const elm = createElement('x-child', { is: Child });
+        const subscribe = mockFn();
+        // Note this follows the shape of the signal implementation
+        // but it's not added as a trusted signal (add using lwc.addTrustedSignal)
+        const signal = {
+            get value() {
+                return 'initial value';
+            },
+            subscribe,
+        };
+        elm.signal = signal;
+        document.body.appendChild(elm);
+        await Promise.resolve();
+
+        expect(subscribe).not.toHaveBeenCalled();
+    });
+
+    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);
+
+        await Promise.resolve();
+
+        expect(elm.shadowRoot.querySelector('h1').textContent).toBe('hello');
+    });
+});
+
+describe('ENABLE_EXPERIMENTAL_SIGNALS not set', () => {
+    beforeAll(() => {
+        setFeatureFlagForTest('ENABLE_EXPERIMENTAL_SIGNALS', false);
+    });
+
+    it('does not subscribe or unsubscribe if feature flag is disabled', async () => {
+        const elm = createElement('x-child', { is: Child });
+        const signal = new Signal('initial value');
+        elm.signal = signal;
+        document.body.appendChild(elm);
+        await Promise.resolve();
+
+        expect(signal.getSubscriberCount()).toBe(0);
+        expect(signal.getRemovedSubscriberCount()).toBe(0);
+
+        document.body.removeChild(elm);
+        await Promise.resolve();
+
+        expect(signal.getSubscriberCount()).toBe(0);
+        expect(signal.getRemovedSubscriberCount()).toBe(0);
+    });
+});

packages/@lwc/integration-not-karma/test/signal-trusted/protocol/x/child/child.html

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

packages/@lwc/integration-not-karma/test/signal-trusted/protocol/x/child/child.js

@@ -0,0 +1,10 @@
+import { LightningElement, api } from 'lwc';
+
+export default class extends LightningElement {
+    @api renderCount = 0;
+    @api signal;
+
+    renderedCallback() {
+        this.renderCount++;
+    }
+}

packages/@lwc/integration-not-karma/test/signal-trusted/protocol/x/container/container.html

@@ -0,0 +1,3 @@
+<template>
+    <slot></slot>
+</template>

packages/@lwc/integration-not-karma/test/signal-trusted/protocol/x/container/container.js

@@ -0,0 +1,9 @@
+import { LightningElement, api } from 'lwc';
+
+export default class extends LightningElement {
+    @api renderCount = 0;
+
+    renderedCallback() {
+        this.renderCount++;
+    }
+}

packages/@lwc/integration-not-karma/test/signal-trusted/protocol/x/duplicateSignalOnTemplate/duplicateSignalOnTemplate.html

@@ -0,0 +1,5 @@
+<template>
+    {signal.value}
+    {signal.value}
+    {signal.value}
+</template>