feat: add freezeTemplate() API, warn on mutation (#2825)

* feat: add freezeTemplate() API, warn on mutation

* fix: warn on slots/renderMode as well

* fix: add comment

* fix: remove duplicate process.env.NODE_ENV check

Author: nolanlawson

packages/@lwc/compiler/src/transformers/template.ts

@@ -70,6 +70,7 @@ function serialize(
         `${namespace}-${name}_${path.basename(filename, path.extname(filename))}`
     );
     let buffer = '';
+    buffer += `import { freezeTemplate } from "lwc";\n\n`;
     buffer += `import _implicitStylesheets from "${cssRelPath}";\n\n`;
     buffer += `import _implicitScopedStylesheets from "${scopedCssRelPath}?scoped=true";\n\n`;
     buffer += code;
@@ -83,6 +84,9 @@ function serialize(
     buffer += 'if (_implicitStylesheets || _implicitScopedStylesheets) {\n';
     buffer += `  tmpl.stylesheetToken = "${scopeToken}"\n`;
     buffer += '}\n';
+    // Note that `renderMode` and `slots` are already rendered in @lwc/template-compiler and appear
+    // as `code` above. At this point, no more expando props should be added to `tmpl`.
+    buffer += 'freezeTemplate(tmpl);\n';
 
     return buffer;
 }

packages/@lwc/engine-core/src/framework/freeze-template.ts

@@ -0,0 +1,128 @@
+/*
+ * Copyright (c) 2018, 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 {
+    ArrayPush,
+    ArraySplice,
+    ArrayUnshift,
+    ArrayCopyWithin,
+    ArrayFill,
+    ArrayPop,
+    ArrayShift,
+    ArraySort,
+    ArrayReverse,
+    defineProperty,
+    getOwnPropertyDescriptor,
+    isUndefined,
+} from '@lwc/shared';
+import { logError } from '../shared/logger';
+import { Template } from './template';
+import { TemplateStylesheetFactories } from './stylesheet';
+
+// See @lwc/engine-core/src/framework/template.ts
+const TEMPLATE_PROPS = ['slots', 'stylesheetToken', 'stylesheets', 'renderMode'] as const;
+
+// Via https://www.npmjs.com/package/object-observer
+const ARRAY_MUTATION_METHODS = [
+    'pop',
+    'push',
+    'shift',
+    'unshift',
+    'reverse',
+    'sort',
+    'fill',
+    'splice',
+    'copyWithin',
+] as const;
+
+function getOriginalArrayMethod(prop: typeof ARRAY_MUTATION_METHODS[number]) {
+    switch (prop) {
+        case 'pop':
+            return ArrayPop;
+        case 'push':
+            return ArrayPush;
+        case 'shift':
+            return ArrayShift;
+        case 'unshift':
+            return ArrayUnshift;
+        case 'reverse':
+            return ArrayReverse;
+        case 'sort':
+            return ArraySort;
+        case 'fill':
+            return ArrayFill;
+        case 'splice':
+            return ArraySplice;
+        case 'copyWithin':
+            return ArrayCopyWithin;
+    }
+}
+
+let mutationWarningsSilenced = false;
+
+// Warn if the user tries to mutate tmpl.stylesheets, e.g.:
+// `tmpl.stylesheets.push(someStylesheetFunction)`
+function warnOnArrayMutation(stylesheets: TemplateStylesheetFactories) {
+    // We can't handle users calling Array.prototype.slice.call(tmpl.stylesheets), but
+    // we can at least warn when they use the most common mutation methods.
+    for (const prop of ARRAY_MUTATION_METHODS) {
+        const originalArrayMethod = getOriginalArrayMethod(prop);
+        stylesheets[prop] = function arrayMutationWarningWrapper() {
+            logError(
+                `Mutating the "stylesheets" array on a template function ` +
+                    `is deprecated and may be removed in a future version of LWC.`
+            );
+            // @ts-ignore
+            return originalArrayMethod.apply(this, arguments);
+        };
+    }
+}
+
+// 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;
+                },
+            });
+        }
+
+        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;
+            },
+        });
+    }
+}

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

@@ -41,6 +41,7 @@ export { profilerControl as __unstable__ProfilerControl } from './profiler';
 export { getUpgradableConstructor } from './upgradable-element';
 export { swapTemplate, swapComponent, swapStyle } from './hot-swaps';
 export { setHooks } from './overridable-hooks';
+export { freezeTemplate } from './freeze-template';
 
 // Experimental or Internal APIs
 export { getComponentConstructor } from './get-component-constructor';

packages/@lwc/engine-core/src/framework/secure-template.ts

@@ -34,6 +34,8 @@ export function registerTemplate(tpl: Template): Template {
     // 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)) {

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

@@ -23,6 +23,7 @@ export {
     setFeatureFlag,
     setFeatureFlagForTest,
     registerTemplate,
+    freezeTemplate,
     registerComponent,
     registerDecorators,
     sanitizeAttribute,

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

@@ -22,6 +22,7 @@ export {
     setFeatureFlag,
     setFeatureFlagForTest,
     registerTemplate,
+    freezeTemplate,
     registerComponent,
     registerDecorators,
     sanitizeAttribute,

packages/@lwc/shared/src/language.ts

@@ -23,15 +23,20 @@ const {
 const { isArray } = Array;
 
 const {
+    copyWithin: ArrayCopyWithin,
+    fill: ArrayFill,
     filter: ArrayFilter,
     find: ArrayFind,
     indexOf: ArrayIndexOf,
     join: ArrayJoin,
     map: ArrayMap,
+    pop: ArrayPop,
     push: ArrayPush,
     reduce: ArrayReduce,
     reverse: ArrayReverse,
+    shift: ArrayShift,
     slice: ArraySlice,
+    sort: ArraySort,
     splice: ArraySplice,
     unshift: ArrayUnshift,
     forEach,
@@ -49,13 +54,18 @@ const {
 export {
     ArrayFilter,
     ArrayFind,
+    ArrayFill,
     ArrayIndexOf,
+    ArrayCopyWithin,
     ArrayJoin,
     ArrayMap,
+    ArrayPop,
     ArrayPush,
     ArrayReduce,
     ArrayReverse,
+    ArrayShift,
     ArraySlice,
+    ArraySort,
     ArraySplice,
     ArrayUnshift,
     assign,

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

@@ -93,23 +93,28 @@ window.TestUtils = (function (lwc, jasmine, beforeAll) {
                         },
                     };
                 },
-                compare: function compare(actual, expectedMessage) {
-                    function matchMessage(message) {
+                compare: function compare(actual, expectedMessages) {
+                    function matchMessage(message, expectedMessage) {
                         if (typeof expectedMessage === 'string') {
                             return message === expectedMessage;
                         } else {
                             return expectedMessage.test(message);
                         }
                     }
 
+                    if (!Array.isArray(expectedMessages)) {
+                        expectedMessages = [expectedMessages];
+                    }
+
                     if (typeof actual !== 'function') {
                         throw new Error('Expected function to throw error.');
                     } else if (
-                        typeof expectedMessage !== 'string' &&
-                        !(expectedMessage instanceof RegExp)
+                        expectedMessages.some(function (message) {
+                            return typeof message !== 'string' && !(message instanceof RegExp);
+                        })
                     ) {
                         throw new Error(
-                            'Expected a string or a RegExp to compare the thrown error against.'
+                            'Expected a string or a RegExp to compare the thrown error against, or an array of such.'
                         );
                     }
 
@@ -147,36 +152,39 @@ window.TestUtils = (function (lwc, jasmine, beforeAll) {
                             return fail(
                                 'Expected console.' +
                                     methodName +
-                                    ' to called once with "' +
-                                    expectedMessage +
-                                    '", but was never called.'
+                                    ' to called with ' +
+                                    JSON.stringify(expectedMessages) +
+                                    ', but was never called.'
                             );
-                        } else if (callsArgs.length === 1) {
-                            var actualMessage = formatConsoleCall(callsArgs[0]);
-
-                            if (!matchMessage(actualMessage)) {
+                        } else {
+                            if (callsArgs.length !== expectedMessages.length) {
                                 return fail(
                                     'Expected console.' +
                                         methodName +
-                                        ' to be called with "' +
-                                        expectedMessage +
-                                        '", but was called with "' +
-                                        actualMessage +
-                                        '".'
+                                        ' to be called ' +
+                                        expectedMessages.length +
+                                        ' time(s), but was called ' +
+                                        callsArgs.length +
+                                        ' time(s).'
                                 );
-                            } else {
-                                return pass();
                             }
-                        } else {
-                            return fail(
-                                'Expected console.' +
-                                    methodName +
-                                    ' to never called, but it was called ' +
-                                    callsArgs.length +
-                                    ' with ' +
-                                    formattedCalls +
-                                    '.'
-                            );
+                            for (var i = 0; i < callsArgs.length; i++) {
+                                var callsArg = callsArgs[i];
+                                var expectedMessage = expectedMessages[i];
+                                var actualMessage = formatConsoleCall(callsArg);
+                                if (!matchMessage(actualMessage, expectedMessage)) {
+                                    return fail(
+                                        'Expected console.' +
+                                            methodName +
+                                            ' to be called with "' +
+                                            expectedMessage +
+                                            '", but was called with "' +
+                                            actualMessage +
+                                            '".'
+                                    );
+                                }
+                            }
+                            return pass();
                         }
                     }
                 },