feat(engine-core): add ENABLE_FROZEN_TEMPLATE flag

Partially addresses #2826
salesforce · Sep 28, 2022 · d61bd55 · d61bd55
1 parent 60ea25a
commit d61bd55
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 64 deletions.
diff --git a/packages/@lwc/engine-core/src/framework/freeze-template.ts b/packages/@lwc/engine-core/src/framework/freeze-template.ts
@@ -17,7 +17,9 @@ import {
     defineProperty,
     getOwnPropertyDescriptor,
     isUndefined,
+    freeze,
 } from '@lwc/shared';
+import features from '@lwc/features';
 import { logError } from '../shared/logger';
 import { Template } from './template';
 import { TemplateStylesheetFactories } from './stylesheet';
@@ -81,48 +83,85 @@ function warnOnArrayMutation(stylesheets: TemplateStylesheetFactories) {
     }
 }
 
-// TODO [#2782]: eventually freezeTemplate() will _actually_ freeze the tmpl object. Today it
-// just warns on mutation.
 export function freezeTemplate(tmpl: Template) {
-    if (process.env.NODE_ENV !== 'production') {
-        if (!isUndefined(tmpl.stylesheets)) {
-            warnOnArrayMutation(tmpl.stylesheets);
-        }
-        for (const prop of TEMPLATE_PROPS) {
-            let value = tmpl[prop];
-            defineProperty(tmpl, prop, {
-                enumerable: true,
-                configurable: true,
-                get() {
-                    return value;
-                },
-                set(newValue) {
-                    if (!mutationWarningsSilenced) {
-                        logError(
-                            `Dynamically setting the "${prop}" property on a template function ` +
-                                `is deprecated and may be removed in a future version of LWC.`
-                        );
-                    }
-                    value = newValue;
-                },
-            });
+    if (features.ENABLE_FROZEN_TEMPLATE) {
+        // Deep freeze the template
+        freeze(tmpl);
+        if ('stylesheets' in tmpl) {
+            freeze(tmpl.stylesheets);
         }
+    } else {
+        // TODO [#2782]: remove this flag and delete the legacy behavior
 
-        const originalDescriptor = getOwnPropertyDescriptor(tmpl, 'stylesheetTokens');
+        // When ENABLE_FROZEN_TEMPLATE is false, then we shim stylesheetTokens on top of stylesheetToken for anyone who
+        // is accessing the old internal API (backwards compat). Details: https://salesforce.quip.com/v1rmAFu2cKAr
         defineProperty(tmpl, 'stylesheetTokens', {
             enumerable: true,
             configurable: true,
-            get: originalDescriptor!.get,
+            get() {
+                const { stylesheetToken } = this;
+                if (isUndefined(stylesheetToken)) {
+                    return stylesheetToken;
+                }
+                // Shim for the old `stylesheetTokens` property
+                // See https://github.com/salesforce/lwc/pull/2332/files#diff-7901555acef29969adaa6583185b3e9bce475cdc6f23e799a54e0018cb18abaa
+                return {
+                    hostAttribute: `${stylesheetToken}-host`,
+                    shadowAttribute: stylesheetToken,
+                };
+            },
+
             set(value) {
-                logError(
-                    `Dynamically setting the "stylesheetTokens" property on a template function ` +
-                        `is deprecated and may be removed in a future version of LWC.`
-                );
-                // Avoid logging twice (for both stylesheetToken and stylesheetTokens)
-                mutationWarningsSilenced = true;
-                originalDescriptor!.set!.call(this, value);
-                mutationWarningsSilenced = false;
+                // If the value is null or some other exotic object, you would be broken anyway in the past
+                // because the engine would try to access hostAttribute/shadowAttribute, which would throw an error.
+                // However it may be undefined in newer versions of LWC, so we need to guard against that case.
+                this.stylesheetToken = isUndefined(value)
+                    ? undefined
+                    : (value as any).shadowAttribute;
             },
         });
+
+        // When ENABLE_FROZEN_TEMPLATE is false, warn in dev mode whenever someone is mutating the template
+        if (process.env.NODE_ENV !== 'production') {
+            if (!isUndefined(tmpl.stylesheets)) {
+                warnOnArrayMutation(tmpl.stylesheets);
+            }
+            for (const prop of TEMPLATE_PROPS) {
+                let value = tmpl[prop];
+                defineProperty(tmpl, prop, {
+                    enumerable: true,
+                    configurable: true,
+                    get() {
+                        return value;
+                    },
+                    set(newValue) {
+                        if (!mutationWarningsSilenced) {
+                            logError(
+                                `Dynamically setting the "${prop}" property on a template function ` +
+                                    `is deprecated and may be removed in a future version of LWC.`
+                            );
+                        }
+                        value = newValue;
+                    },
+                });
+            }
+
+            const originalDescriptor = getOwnPropertyDescriptor(tmpl, 'stylesheetTokens');
+            defineProperty(tmpl, 'stylesheetTokens', {
+                enumerable: true,
+                configurable: true,
+                get: originalDescriptor!.get,
+                set(value) {
+                    logError(
+                        `Dynamically setting the "stylesheetTokens" property on a template function ` +
+                            `is deprecated and may be removed in a future version of LWC.`
+                    );
+                    // Avoid logging twice (for both stylesheetToken and stylesheetTokens)
+                    mutationWarningsSilenced = true;
+                    originalDescriptor!.set!.call(this, value);
+                    mutationWarningsSilenced = false;
+                },
+            });
+        }
     }
 }
diff --git a/packages/@lwc/engine-core/src/framework/secure-template.ts b/packages/@lwc/engine-core/src/framework/secure-template.ts
@@ -4,7 +4,6 @@
  * SPDX-License-Identifier: MIT
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
  */
-import { defineProperty, isUndefined } from '@lwc/shared';
 import { Template } from './template';
 import { checkVersionMismatch } from './check-version-mismatch';
 
@@ -29,34 +28,6 @@ export function registerTemplate(tpl: Template): Template {
     }
     signedTemplateSet.add(tpl);
 
-    // FIXME[@W-10950976]: the template object should be frozen, and it should not be possible to set
-    // the stylesheets or stylesheetToken(s). For backwards compat, though, we shim stylesheetTokens
-    // on top of stylesheetToken for anyone who is accessing the old internal API.
-    // Details: https://salesforce.quip.com/v1rmAFu2cKAr
-    defineProperty(tpl, 'stylesheetTokens', {
-        enumerable: true,
-        configurable: true,
-        get() {
-            const { stylesheetToken } = this;
-            if (isUndefined(stylesheetToken)) {
-                return stylesheetToken;
-            }
-            // Shim for the old `stylesheetTokens` property
-            // See https://github.com/salesforce/lwc/pull/2332/files#diff-7901555acef29969adaa6583185b3e9bce475cdc6f23e799a54e0018cb18abaa
-            return {
-                hostAttribute: `${stylesheetToken}-host`,
-                shadowAttribute: stylesheetToken,
-            };
-        },
-
-        set(value) {
-            // If the value is null or some other exotic object, you would be broken anyway in the past
-            // because the engine would try to access hostAttribute/shadowAttribute, which would throw an error.
-            // However it may be undefined in newer versions of LWC, so we need to guard against that case.
-            this.stylesheetToken = isUndefined(value) ? undefined : (value as any).shadowAttribute;
-        },
-    });
-
     // chaining this method as a way to wrap existing
     // assignment of templates easily, without too much transformation
     return tpl;

diff --git a/packages/@lwc/features/src/flags.ts b/packages/@lwc/features/src/flags.ts
@@ -21,6 +21,7 @@ const features: FeatureFlagMap = {
     ENABLE_REACTIVE_SETTER: null,
     ENABLE_WIRE_SYNC_EMIT: null,
     ENABLE_LIGHT_GET_ROOT_NODE_PATCH: null,
+    ENABLE_FROZEN_TEMPLATE: null,
 };
 
 if (!globalThis.lwcRuntimeFlags) {

diff --git a/packages/@lwc/features/src/types.ts b/packages/@lwc/features/src/types.ts
@@ -105,6 +105,16 @@ export interface FeatureFlagMap {
      *  - `Node.prototype.getRootNode`
      */
     ENABLE_LIGHT_GET_ROOT_NODE_PATCH: FeatureFlagValue;
+
+    /**
+     * Flag to enable the "frozen template" feature. With this flag enabled, the template object
+     * imported from HTML files is frozen and cannot be modified. E.g. this will throw:
+     * ```js
+     * import template from './template.html';
+     * template.stylesheets = [];
+     * ```
+     */
+    ENABLE_FROZEN_TEMPLATE: FeatureFlagValue;
 }
 
 export type FeatureFlagName = keyof FeatureFlagMap;
diff --git a/packages/@lwc/integration-karma/test/api/freezeTemplate/index.spec.js b/packages/@lwc/integration-karma/test/api/freezeTemplate/index.spec.js
@@ -1,4 +1,4 @@
-import { registerTemplate, freezeTemplate } from 'lwc';
+import { registerTemplate, freezeTemplate, setFeatureFlagForTest } from 'lwc';
 
 describe('freezeTemplate', () => {
     it('should warn when setting tmpl.stylesheetToken', () => {
@@ -153,4 +153,33 @@ describe('freezeTemplate', () => {
             expect(descriptor.configurable).toEqual(true);
         }
     });
+
+    describe('ENABLE_FROZEN_TEMPLATE set to true', () => {
+        beforeEach(() => {
+            setFeatureFlagForTest('ENABLE_FROZEN_TEMPLATE', true);
+        });
+
+        afterAll(() => {
+            setFeatureFlagForTest('ENABLE_FROZEN_TEMPLATE', false);
+        });
+
+        it('deep-freezes the template', () => {
+            const template = registerTemplate(() => []);
+            const stylesheet = () => 'div { color: red }';
+            template.stylesheets = [stylesheet];
+            template.stylesheetToken = 'myToken';
+            freezeTemplate(template);
+
+            expect(Object.isFrozen(template)).toEqual(true);
+            expect(Object.isFrozen(template.stylesheets)).toEqual(true);
+        });
+
+        it('freezes a template with no stylesheets', () => {
+            const template = registerTemplate(() => []);
+            freezeTemplate(template);
+
+            expect(Object.isFrozen(template)).toEqual(true);
+            expect(template.stylesheets).toEqual(undefined);
+        });
+    });
 });