Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions packages/@lwc/engine-core/src/framework/modules/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ArrayFilter,
ContextEventName,
isTrustedContext,
legacyIsTrustedContext,
type ContextProvidedCallback,
type ContextBinding as IContextBinding,
} from '@lwc/shared';
Expand Down Expand Up @@ -92,7 +93,17 @@ export function connectContext(vm: VM) {

const enumerableKeys = keys(getPrototypeOf(component));
const contextfulKeys = ArrayFilter.call(enumerableKeys, (enumerableKey) =>
isTrustedContext((component as any)[enumerableKey])
/**
* The legacy validation behavior was that this check should only
* be performed for runtimes that have provided a trustedContext set.
* However, this resulted in a bug as all component properties were
* being considered context in environments where the trustedContext
* set had not been provided. The runtime flag has been added as a killswitch
* in case the fix needs to be reverted.
*/
lwcRuntimeFlags.ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION
? legacyIsTrustedContext((component as any)[enumerableKey])
: isTrustedContext((component as any)[enumerableKey])
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhefferman-sfdc - Just to confirm, this is how it will work with the killswitch right?

killswitch on => ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is off

killswitch off => ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is on

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmsjtu ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is the kill switch so:

killswitch on => ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is on => use current legacy validation which were are fixing (legacyIsTrustedSignal/Context)

killswitch off => ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is off => use corrected validation (isTrustedSignal/Context)

Is that OK / makes sense?

Copy link
Contributor Author

@jhefferman-sfdc jhefferman-sfdc Sep 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmsjtu you are correct, but I think we will have to invert the flag in core as our default flag state is undefined and we want the default behavior to be the corrected behavior? Aka what you said:

killswitch on => ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is off

killswitch off => ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION is on


if (contextfulKeys.length === 0) {
Expand Down Expand Up @@ -128,7 +139,17 @@ export function disconnectContext(vm: VM) {

const enumerableKeys = keys(getPrototypeOf(component));
const contextfulKeys = ArrayFilter.call(enumerableKeys, (enumerableKey) =>
isTrustedContext((component as any)[enumerableKey])
/**
* The legacy validation behavior was that this check should only
* be performed for runtimes that have provided a trustedContext set.
* However, this resulted in a bug as all component properties were
* being considered context in environments where the trustedContext
* set had not been provided. The runtime flag has been added as a killswitch
* in case the fix needs to be reverted.
*/
lwcRuntimeFlags.ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION
? legacyIsTrustedContext((component as any)[enumerableKey])
: isTrustedContext((component as any)[enumerableKey])
);

if (contextfulKeys.length === 0) {
Expand Down
21 changes: 17 additions & 4 deletions packages/@lwc/engine-core/src/framework/mutation-tracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 component properties 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)) ||
(!lwcRuntimeFlags.ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION && isTrustedSignal(target))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(lwcRuntimeFlags.ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION &&
legacyIsTrustedSignal(target)) ||
(!lwcRuntimeFlags.ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION && isTrustedSignal(target))
lwcRuntimeFlags.ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION &&
? legacyIsTrustedSignal(target)
: isTrustedSignal(target)

bruv

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a slight preference for the verbose syntax but on second thoughts, you're right

) {
// 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));
}
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/features/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions packages/@lwc/features/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 8 additions & 2 deletions packages/@lwc/shared/src/__tests__/context.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ describe('context', () => {
let setTrustedContextSet: (signals: WeakSet<object>) => void;
let addTrustedContext: (signal: object) => void;
let isTrustedContext: (target: object) => boolean;
let legacyIsTrustedContext: (target: object) => boolean;

beforeEach(async () => {
vi.resetModules();
Expand All @@ -21,6 +22,7 @@ describe('context', () => {
setTrustedContextSet = contextModule.setTrustedContextSet;
addTrustedContext = contextModule.addTrustedContext;
isTrustedContext = contextModule.isTrustedContext;
legacyIsTrustedContext = contextModule.legacyIsTrustedContext;
});

it('should set and get context keys', () => {
Expand Down Expand Up @@ -96,8 +98,12 @@ describe('context', () => {
expect(isTrustedContext({})).toBe(false);
});

it('should return true for all calls when trustedContexts is not set', () => {
expect(isTrustedContext({})).toBe(true);
it('should return false for all calls when trustedContexts is not set', () => {
expect(isTrustedContext({})).toBe(false);
});

it('legacyIsTrustedContext should return true when trustedContexts is not set', () => {
expect(legacyIsTrustedContext({})).toBe(true);
Comment on lines +101 to +106
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For posterity, can we add a test that shows the legacy vs new beavior?

Ex, a test where the flag is enabled vs disabled and the expected outputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I've added some tests around this change.

});
});
});
10 changes: 8 additions & 2 deletions packages/@lwc/shared/src/__tests__/signals.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
});
});
});
19 changes: 17 additions & 2 deletions packages/@lwc/shared/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,30 @@ export function setTrustedContextSet(context: WeakSet<object>) {
}

export function addTrustedContext(contextParticipant: object) {
// This should be a no-op when the trustedSignals set isn't set by runtime
// This should be a no-op when the trustedContext set isn't set by runtime
trustedContext?.add(contextParticipant);
}

export function isTrustedContext(target: object): boolean {
/**
* The legacy validation behavior was that this check should only
* be performed for runtimes that have provided a trustedContext set.
* However, this resulted in a bug as all component properties were
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: technically it was only object values, not all values

* being considered context in environments where the trustedContext
* set had not been provided. The runtime flag has been added as a killswitch
* in case the fix needs to be reverted.
*/
export function legacyIsTrustedContext(target: object): boolean {
if (!trustedContext) {
// The runtime didn't set a trustedContext set
// this check should only be performed for runtimes that care about filtering context participants to track
return true;
}
return trustedContext.has(target);
}

export function isTrustedContext(target: object): boolean {
if (!trustedContext) {
return false;
}
return trustedContext.has(target);
}
17 changes: 16 additions & 1 deletion packages/@lwc/shared/src/signals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 component properties 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
Expand All @@ -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);
}
6 changes: 5 additions & 1 deletion packages/@lwc/ssr-runtime/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
type ContextProvidedCallback,
type ContextBinding as IContextBinding,
isTrustedContext,
legacyIsTrustedContext,
getContextKeys,
isUndefined,
keys,
Expand Down Expand Up @@ -66,8 +67,11 @@ export function connectContext(le: LightningElement) {
const { connectContext } = contextKeys;

const enumerableKeys = keys(le);

const contextfulKeys = ArrayFilter.call(enumerableKeys, (enumerableKey) =>
isTrustedContext((le as any)[enumerableKey])
lwcRuntimeFlags.ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION
? legacyIsTrustedContext((le as any)[enumerableKey])
: isTrustedContext((le as any)[enumerableKey])
);

if (contextfulKeys.length === 0) {
Expand Down
2 changes: 1 addition & 1 deletion scripts/bundlesize/bundlesize.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"files": [
{
"path": "packages/@lwc/engine-dom/dist/index.js",
"maxSize": "24.68KB"
"maxSize": "24.72KB"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

},
{
"path": "packages/@lwc/synthetic-shadow/dist/index.js",
Expand Down
Loading