fix: gate protection, review comments, malformed context handling

Author: jhefferman-sfdc

packages/@lwc/engine-core/README.md

@@ -121,3 +121,20 @@ This experimental API enables the removal of an object's observable membrane pro
 This experimental API enables the addition of a signal as a trusted signal. If the [ENABLE_EXPERIMENTAL_SIGNALS](https://github.com/salesforce/lwc/blob/master/packages/%40lwc/features/README.md#lwcfeatures) feature is enabled, any signal value change will trigger a re-render.
 
 If `setTrustedSignalSet` is called more than once, it will throw an error. If it is never called, then no trusted signal validation will be performed. The same `setTrustedSignalSet` API must be called on both `@lwc/engine-dom` and `@lwc/signals`.
+
+### setTrustedContextSet()
+
+This experimental API enables the addition of context as trusted context. If the [ENABLE_EXPERIMENTAL_SIGNALS](https://github.com/salesforce/lwc/blob/master/packages/%40lwc/features/README.md#lwcfeatures) feature is enabled
+and context has been added to this set, the context object's connectContext and disconnectContext symbols will be called with a ContextConnector when the associated component is connected and disconnected.
+
+If `setTrustedContextSet` is called more than once, it will throw an error. If it is never called, then context will not be connected.
+
+### ContextConnector
+
+The context manager `connectContext` and `disconnectContext` symbols are called with this object when contextful components are connected and disconnected. The ContextConnector exposes `provideContext` and `consumeContext`,
+enabling the provision/consumption of a contextful Signal of a specified variety for the associated component.
+
+### setContextKeys
+
+Enables a state manager context implementation to provide LWC with context Symbols, namely `connectContext` and `disconnectContext`. These symbols would then be defined on any context manager implementation, and will be called
+with a ContextConnector object when contextful components are connected and disconnected.

packages/@lwc/engine-core/src/framework/modules/context.ts

@@ -12,17 +12,18 @@ import {
     ArrayFilter,
     ContextEventName,
     isTrustedContext,
+    type ContextProvidedCallback,
+    type ContextConnector as IContextConnector,
 } from '@lwc/shared';
 import { type VM } from '../vm';
-import { logErrorOnce } from '../../shared/logger';
+import { logWarnOnce } from '../../shared/logger';
 import type { Signal } from '@lwc/signals';
 import type { RendererAPI } from '../renderer';
 import type { ShouldContinueBubbling } from '../wiring/types';
 
-type ContextProvidedCallback = (contextSignal?: Signal<unknown>) => void;
 type ContextVarieties = Map<unknown, Signal<unknown>>;
 
-class ContextConnector<C extends object> {
+class ContextConnector<C extends object> implements IContextConnector<C> {
     component: C;
     #renderer: RendererAPI;
     #providedContextVarieties: ContextVarieties;
@@ -54,7 +55,7 @@ class ContextConnector<C extends object> {
         }
 
         if (this.#providedContextVarieties.has(contextVariety)) {
-            logErrorOnce(
+            logWarnOnce(
                 'Multiple contexts of the same variety were provided. Only the first context will be used.'
             );
             return;
@@ -103,9 +104,17 @@ export function connectContext(vm: VM) {
 
     const providedContextVarieties: ContextVarieties = new Map();
 
-    for (let i = 0; i < contextfulKeys.length; i++) {
-        (component as any)[contextfulKeys[i]][connectContext](
-            new ContextConnector(vm, component, providedContextVarieties)
+    try {
+        for (let i = 0; i < contextfulKeys.length; i++) {
+            (component as any)[contextfulKeys[i]][connectContext](
+                new ContextConnector(vm, component, providedContextVarieties)
+            );
+        }
+    } catch (err: any) {
+        logWarnOnce(
+            `Attempted to connect to trusted context but received the following error: ${
+                err.message
+            }`
         );
     }
 }
@@ -129,7 +138,15 @@ export function disconnectContext(vm: VM) {
         return;
     }
 
-    for (let i = 0; i < contextfulKeys.length; i++) {
-        (component as any)[contextfulKeys[i]][disconnectContext](component);
+    try {
+        for (let i = 0; i < contextfulKeys.length; i++) {
+            (component as any)[contextfulKeys[i]][disconnectContext](component);
+        }
+    } catch (err: any) {
+        logWarnOnce(
+            `Attempted to disconnect from trusted context but received the following error: ${
+                err.message
+            }`
+        );
     }
 }

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

@@ -42,6 +42,7 @@ export function componentValueObserved(vm: VM, key: PropertyKey, target: any = {
         isObject(target) &&
         !isNull(target) &&
         isTrustedSignal(target) &&
+        process.env.IS_BROWSER &&
         // Only subscribe if a template is being rendered by the engine
         tro.isObserving()
     ) {

packages/@lwc/engine-core/src/framework/vm.ts

@@ -704,8 +704,10 @@ export function runConnectedCallback(vm: VM) {
         connectWireAdapters(vm);
     }
 
-    // Setup context before connected callback is executed
-    connectContext(vm);
+    if (lwcRuntimeFlags.ENABLE_EXPERIMENTAL_SIGNALS) {
+        // Setup context before connected callback is executed
+        connectContext(vm);
+    }
 
     const { connectedCallback } = vm.def;
     if (!isUndefined(connectedCallback)) {
@@ -756,7 +758,11 @@ function runDisconnectedCallback(vm: VM) {
     if (process.env.NODE_ENV !== 'production') {
         assert.isTrue(vm.state !== VMState.disconnected, `${vm} must be inserted.`);
     }
-    disconnectContext(vm);
+
+    if (lwcRuntimeFlags.ENABLE_EXPERIMENTAL_SIGNALS) {
+        disconnectContext(vm);
+    }
+
     if (isFalse(vm.isDirty)) {
         // this guarantees that if the component is reused/reinserted,
         // it will be re-rendered because we are disconnecting the reactivity

packages/@lwc/engine-server/src/index.ts

@@ -29,7 +29,6 @@ export {
     isComponentConstructor,
     parseFragment,
     parseFragment as parseSVGFragment,
-    setTrustedSignalSet,
     setTrustedContextSet,
     setContextKeys,
 } from '@lwc/engine-core';

packages/@lwc/integration-karma/helpers/test-hydrate.js

@@ -22,13 +22,22 @@ window.HydrateTest = (function (lwc, testUtils) {
         return div;
     }
 
+    function setFeatureFlags(requiredFeatureFlags, value) {
+        if (requiredFeatureFlags) {
+            requiredFeatureFlags.forEach((featureFlag) => {
+                lwc.setFeatureFlagForTest(featureFlag, value);
+            });
+        }
+    }
+
     function runTest(ssrRendered, Component, testConfig) {
         const container = appendTestTarget(ssrRendered);
         const selector = container.firstChild.tagName.toLowerCase();
         let target = container.querySelector(selector);
 
         let testResult;
         const consoleSpy = testUtils.spyConsole();
+        setFeatureFlags(testConfig.requiredFeatureFlags, true);
 
         if (testConfig.test) {
             const snapshot = testConfig.snapshot ? testConfig.snapshot(target) : {};

packages/@lwc/integration-karma/helpers/test-utils.js

@@ -470,6 +470,10 @@ window.TestUtils = (function (lwc, jasmine, beforeAll) {
         }
     }
 
+    function setFeatureFlagForTest(featureFlag, value) {
+        LWC.setFeatureFlagForTest(featureFlag, value);
+    }
+
     // This mapping should be kept up-to-date with the mapping in @lwc/shared -> aria.ts
     const ariaPropertiesMapping = {
         ariaAutoComplete: 'aria-autocomplete',
@@ -747,6 +751,7 @@ window.TestUtils = (function (lwc, jasmine, beforeAll) {
         catchUnhandledRejectionsAndErrors,
         addTrustedSignal,
         expectEquivalentDOM,
+        setFeatureFlagForTest,
         ...apiFeatures,
     };
 })(LWC, jasmine, beforeAll);

packages/@lwc/integration-karma/scripts/karma-plugins/hydration-tests.js

@@ -11,16 +11,16 @@ const fs = require('node:fs/promises');
 const { format } = require('node:util');
 const { rollup } = require('rollup');
 const lwcRollupPlugin = require('@lwc/rollup-plugin');
-const ssr = ENGINE_SERVER ? require('@lwc/engine-server') : require('@lwc/ssr-runtime');
+const lwcSsr = ENGINE_SERVER ? require('@lwc/engine-server') : require('@lwc/ssr-runtime');
 const { DISABLE_STATIC_CONTENT_OPTIMIZATION } = require('../shared/options');
 const Watcher = require('./Watcher');
 
 const context = {
-    LWC: ssr,
+    LWC: lwcSsr,
     moduleOutput: null,
 };
 
-ssr.setHooks({
+lwcSsr.setHooks({
     sanitizeHtmlContent(content) {
         return content;
     },
@@ -145,14 +145,7 @@ function throwOnUnexpectedConsoleCalls(runnable, expectedConsoleCalls = {}) {
  * So, script runs, generates markup, & we get that markup out and return it to Karma for use
  * in client-side tests.
  */
-async function getSsrCode(moduleCode, testConfig, filename) {
-    // Create a temporary module to evaluate the bundled code and extract the expected console calls
-    const configModule = new vm.Script(testConfig);
-    const configContext = { config: {} };
-    vm.createContext(configContext);
-    configModule.runInContext(configContext);
-    const { expectedSSRConsoleCalls } = configContext.config;
-
+async function getSsrCode(moduleCode, testConfig, filename, expectedSSRConsoleCalls) {
     const script = new vm.Script(
         `
             ${testConfig};
@@ -221,10 +214,22 @@ function createHCONFIG2JSPreprocessor(config, logger, emitter) {
         // Wrap all the tests into a describe block with the file stricture name
         const describeTitle = path.relative(basePath, suiteDir).split(path.sep).join(' ');
 
-        try {
-            const { code: testCode, watchFiles: testWatchFiles } =
-                await getTestModuleCode(filePath);
+        const { code: testCode, watchFiles: testWatchFiles } = await getTestModuleCode(filePath);
 
+        // Create a temporary module to evaluate the bundled code and extract config properties for test configuration
+        const configModule = new vm.Script(testCode);
+        const configContext = { config: {} };
+        vm.createContext(configContext);
+        configModule.runInContext(configContext);
+        const { expectedSSRConsoleCalls, requiredFeatureFlags } = configContext.config;
+
+        if (requiredFeatureFlags) {
+            requiredFeatureFlags.forEach((featureFlag) => {
+                lwcSsr.setFeatureFlagForTest(featureFlag, true);
+            });
+        }
+
+        try {
             // You can add an `.only` file alongside an `index.spec.js` file to make it `fdescribe()`
             const onlyFileExists = await existsUp(suiteDir, '.only');
 
@@ -241,7 +246,8 @@ function createHCONFIG2JSPreprocessor(config, logger, emitter) {
                 ssrOutput = await getSsrCode(
                     componentDefCSR,
                     testCode,
-                    path.join(suiteDir, 'ssr.js')
+                    path.join(suiteDir, 'ssr.js'),
+                    expectedSSRConsoleCalls
                 );
             } else {
                 // ssr-compiler has it's own def
@@ -254,7 +260,8 @@ function createHCONFIG2JSPreprocessor(config, logger, emitter) {
                 ssrOutput = await getSsrCode(
                     componentDefSSR.replace(`process.env.NODE_ENV === 'test-karma-lwc'`, 'true'),
                     testCode,
-                    path.join(suiteDir, 'ssr.js')
+                    path.join(suiteDir, 'ssr.js'),
+                    expectedSSRConsoleCalls
                 );
             }
 
@@ -271,6 +278,12 @@ function createHCONFIG2JSPreprocessor(config, logger, emitter) {
             const location = path.relative(basePath, filePath);
             log.error('Error processing “%s”\n\n%s\n', location, error.stack || error.message);
             done(error, null);
+        } finally {
+            if (requiredFeatureFlags) {
+                requiredFeatureFlags.forEach((featureFlag) => {
+                    lwcSsr.setFeatureFlagForTest(featureFlag, false);
+                });
+            }
         }
     };
 }

packages/@lwc/integration-karma/test-hydration/context/index.spec.js

@@ -1,11 +1,13 @@
 export default {
     // server is expected to generate the same console error as the client
     expectedSSRConsoleCalls: {
-        error: [
+        error: [],
+        warn: [
+            'Attempted to connect to trusted context but received the following error',
             'Multiple contexts of the same variety were provided. Only the first context will be used.',
         ],
-        warn: [],
     },
+    requiredFeatureFlags: ['ENABLE_EXPERIMENTAL_SIGNALS'],
     snapshot(target) {
         const grandparent = target.shadowRoot.querySelector('x-grandparent');
         const detachedChild = target.shadowRoot.querySelector('x-child');
@@ -36,12 +38,14 @@ export default {
         assertContextDisconnected(target, snapshot);
 
         // Expect an error as one context was generated twice.
+        // Expect an error as one context was malformed (did not define connectContext or disconnectContext methods).
         // Expect server/client context output parity (no hydration warnings)
         TestUtils.expectConsoleCalls(consoleCalls, {
-            error: [
+            error: [],
+            warn: [
+                'Attempted to connect to trusted context but received the following error',
                 'Multiple contexts of the same variety were provided. Only the first context will be used.',
             ],
-            warn: [],
         });
     },
 };
@@ -52,7 +56,7 @@ function assertCorrectContext(snapshot) {
             .withContext(`${component.tagName} should have the correct context`)
             .toBe('grandparent provided value, another grandparent provided value');
 
-        expect(component.context.connectProvidedComponent.hostElement)
+        expect(component.context.connectProvidedComponent?.hostElement)
             .withContext(
                 `The context of ${component.tagName} should have been connected with the correct component`
             )
@@ -86,7 +90,7 @@ function assertContextDisconnected(target, snapshot) {
     Object.values(snapshot.components).forEach(
         (component) =>
             (component.disconnect = () => {
-                expect(component.context.disconnectProvidedComponent.hostElement)
+                expect(component.context.disconnectProvidedComponent?.hostElement)
                     .withContext(
                         `The context of ${component.tagName} should have been disconnected with the correct component`
                     )

packages/@lwc/integration-karma/test-hydration/context/x/contextManager/contextManager.js

@@ -35,8 +35,19 @@ class MockContextSignal {
     }
 }
 
+// This is a malformed context signal that does not implement the connectContext or disconnectContext methods
+class MockMalformedContextSignal {
+    constructor() {
+        trustedContext.add(this);
+    }
+}
+
 export const defineContext = (fromContext) => {
     const contextDefinition = (initialValue) =>
         new MockContextSignal(initialValue, contextDefinition, fromContext);
     return contextDefinition;
 };
+
+export const defineMalformedContext = () => {
+    return () => new MockMalformedContextSignal();
+};