Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
110 changes: 110 additions & 0 deletions packages/@lwc/engine-core/src/framework/__tests__/context.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* Copyright (c) 2025, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { describe, it, expect, vi, beforeAll, afterEach, afterAll } from 'vitest';
import { setTrustedContextSet, setContextKeys } from '@lwc/shared';
import { logWarnOnce } from '../../shared/logger';
import { connectContext, disconnectContext } from '../modules/context';

// Mock the logger to inspect console output during tests
vi.mock('../../shared/logger', () => ({
logWarnOnce: vi.fn(),
}));
Comment on lines +13 to +15
Copy link
Member

Choose a reason for hiding this comment

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

We have some helper methods defined already for the loggers you could use.


// Create mock component with a regular, non-contextful property
const mockComponent = {};
Object.setPrototypeOf(mockComponent, {
regularProp: 'not contextful',
});

// Create mock renderer
const mockRenderer = {
registerContextProvider: vi.fn(),
};

// Create mock VM
const mockVM = {
component: mockComponent,
elm: null,
renderer: mockRenderer,
} as any;

if (!(globalThis as any).lwcRuntimeFlags) {
Object.defineProperty(globalThis, 'lwcRuntimeFlags', { value: {} });
}

/**
* Need to be able to set and reset the flags at will (lwc/features doesn't provide this)
*/
const setFeatureFlag = (name: string, value: boolean) => {
(globalThis as any).lwcRuntimeFlags[name] = value;
};
Comment on lines +42 to +44
Copy link
Member

Choose a reason for hiding this comment

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

You can re-use the setFeatureFlagsForTest helper instead of defining your own.


/**
* These tests test that properties are correctly validated within the connectContext and disconnectContext
* functions regardless of whether trusted context has been defined or not.
* Integration tests have been used for extensive coverage of the LWC context feature, but this particular
* scenario is best isolated and unit tested as it involves manipulation of the trusted context API.
* See bug fix: #5492
*/
describe('context functions', () => {
beforeAll(() => {
const connectContext = Symbol('connectContext');
const disconnectContext = Symbol('disconnectContext');
setContextKeys({ connectContext, disconnectContext });
});

afterEach(() => {
vi.clearAllMocks();
});

afterAll(() => {
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false);
});

describe('without setting trusted context', () => {
it('should log a warning when trustedContext is not defined and connectContext is called with legacy signal context validation', () => {
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', true);
connectContext(mockVM);
expect(logWarnOnce).toHaveBeenCalledWith(
'Attempted to connect to trusted context but received the following error: component[contextfulKeys[i]][connectContext2] is not a function'
);
});
Comment on lines +68 to +75
Copy link
Member

Choose a reason for hiding this comment

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

Instead of creating these two tests in context/mutation-tracker how would you feel if we created a single test using an actual LWC to replicate the before and after behavior?

Is it possible to toggle the trusted signal set in that way?


it('should not log a warning when trustedContext is not defined and connectContext is called with non-legacy context validation', () => {
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false);
connectContext(mockVM);
expect(logWarnOnce).not.toHaveBeenCalled();
});

it('should log a warning when trustedContext is not defined and disconnectContext is called with legacy signal context validation', () => {
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', true);
disconnectContext(mockVM);
expect(logWarnOnce).toHaveBeenCalledWith(
'Attempted to disconnect from trusted context but received the following error: component[contextfulKeys[i]][disconnectContext2] is not a function'
);
});

it('should not log a warning when trustedContext is not defined and disconnectContext is called with non-legacy context validation', () => {
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false);
disconnectContext(mockVM);
expect(logWarnOnce).not.toHaveBeenCalled();
});
});

describe('with trusted context set', () => {
it('should not log warnings when trustedContext is defined', () => {
setTrustedContextSet(new WeakSet());
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', true);
connectContext(mockVM);
disconnectContext(mockVM);
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false);
connectContext(mockVM);
disconnectContext(mockVM);
expect(logWarnOnce).not.toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* Copyright (c) 2019, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { describe, it, expect, vi, afterEach, beforeEach, afterAll, beforeAll } from 'vitest';
import { setTrustedSignalSet } from '@lwc/shared';
import { componentValueObserved } from '../../framework/mutation-tracker';

// Create a mock VM object with required properties
const mockVM = {
component: {},
tro: {
isObserving: () => true,
},
} as any;

if (!(globalThis as any).lwcRuntimeFlags) {
Object.defineProperty(globalThis, 'lwcRuntimeFlags', { value: {} });
}

/**
* Need to be able to set and reset the flags at will (lwc/features doesn't provide this)
*/
const setFeatureFlag = (name: string, value: boolean) => {
(globalThis as any).lwcRuntimeFlags[name] = value;
};

/**
* These tests check that properties are correctly validated within the mutation-tracker
* regardless of whether trusted context has been defined by a state manager or not.
* Integration tests have been used for extensive coverage of the LWC signals feature, but this particular
* scenario is best isolated and unit tested as it involves manipulation of the trusted context API.
* See bug fix: #5492
*/
describe('mutation-tracker', () => {
describe('trustedSignal set not defined', () => {
it('should not throw when componentValueObserved is called using the new signals validation', () => {
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false);
expect(() => {
componentValueObserved(mockVM, 'testKey', {});
}).not.toThrow();
});

it('should throw when componentValueObserved is called using legacy signals validation', () => {
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', true);
expect(() => {
componentValueObserved(mockVM, 'testKey', {});
}).toThrow();
});
});

describe('trustedSignal set defined', () => {
it('should not throw when componentValueObserved is called, regardless of validation type', () => {
setTrustedSignalSet(new WeakSet());
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false);
expect(() => {
componentValueObserved(mockVM, 'testKey', {});
}).not.toThrow();

setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', true);
expect(() => {
componentValueObserved(mockVM, 'testKey', {});
}).not.toThrow();
});
});

beforeAll(() => {
setFeatureFlag('ENABLE_EXPERIMENTAL_SIGNALS', true);
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', true);
});

afterAll(() => {
setFeatureFlag('ENABLE_EXPERIMENTAL_SIGNALS', false);
setFeatureFlag('ENABLE_LEGACY_SIGNAL_CONTEXT_VALIDATION', false);
});

beforeEach(() => {
vi.stubEnv('IS_BROWSER', 'true'); // Signals is a browser-only feature
});

afterEach(() => {
vi.unstubAllEnvs(); // Reset environment variables after each test
});
});
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 object values 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 object values 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 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));
}
}
}

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 object values 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.
*/
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);
}
Loading