fix(engine-dom): use native lifecycle callbacks (#3904)

* fix(engine-dom): use native lifecycle callbacks * fix: erroneous comment * fix: only run synthetic checks in synthetic mode * test: use const instead of function * Update packages/@lwc/engine-core/src/framework/rendering.ts Co-authored-by: James Tu <jmsjtu@gmail.com> * chore: force ci --------- Co-authored-by: James Tu <jmsjtu@gmail.com> Co-authored-by: James Tu <j.tu@salesforce.com>
salesforce · Jan 17, 2024 · 3235744 · 3235744
1 parent e1c8085
commit 3235744
Show file tree

Hide file tree

Showing 32 changed files with 262 additions and 133 deletions.
diff --git a/.github/workflows/karma.yml b/.github/workflows/karma.yml
@@ -55,8 +55,6 @@ jobs:
       - run: DISABLE_SYNTHETIC=1 yarn sauce:ci
       - run: LEGACY_BROWSERS=1 yarn sauce:ci
       - run: FORCE_NATIVE_SHADOW_MODE_FOR_TEST=1 yarn sauce:ci
-      - run: ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE=1 yarn sauce:ci
-      - run: ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE=1 DISABLE_SYNTHETIC=1 yarn sauce:ci
       - run: API_VERSION=58 yarn sauce:ci
       - run: API_VERSION=58 DISABLE_SYNTHETIC=1 yarn sauce:ci
       - run: API_VERSION=59 yarn sauce:ci

diff --git a/packages/@lwc/engine-core/src/framework/renderer.ts b/packages/@lwc/engine-core/src/framework/renderer.ts
@@ -62,7 +62,11 @@ export interface RendererAPI {
     isConnected: (node: N) => boolean;
     insertStylesheet: (content: string, target?: ShadowRoot) => void;
     assertInstanceOfHTMLElement: (elm: any, msg: string) => void;
-    createCustomElement: (tagName: string, upgradeCallback: LifecycleCallback) => E;
+    createCustomElement: (
+        tagName: string,
+        upgradeCallback: LifecycleCallback,
+        useNativeLifecycle: boolean
+    ) => E;
     defineCustomElement: (tagName: string) => void;
     ownerDocument(elm: E): Document;
     registerContextConsumer: (

diff --git a/packages/@lwc/engine-core/src/framework/rendering.ts b/packages/@lwc/engine-core/src/framework/rendering.ts
@@ -24,7 +24,7 @@ import {
 import { logError } from '../shared/logger';
 import { getComponentTag } from '../shared/format';
 import { RendererAPI } from './renderer';
-import { EmptyArray } from './utils';
+import { EmptyArray, shouldUseNativeCustomElementLifecycle } from './utils';
 import { markComponentAsDirty } from './component';
 import { getScopeTokenClass } from './stylesheet';
 import { lockDomMutation, patchElementWithRestrictions, unlockDomMutation } from './restrictions';
@@ -69,6 +69,7 @@ import { applyStaticClassAttribute } from './modules/static-class-attr';
 import { applyStaticStyleAttribute } from './modules/static-style-attr';
 import { applyRefs } from './modules/refs';
 import { applyStaticParts } from './modules/static-parts';
+import { LightningElementConstructor } from './base-lightning-element';
 
 export function patchChildren(
     c1: VNodes,
@@ -308,7 +309,7 @@ function mountCustomElement(
     anchor: Node | null,
     renderer: RendererAPI
 ) {
-    const { sel, owner } = vnode;
+    const { sel, owner, ctor } = vnode;
     const { createCustomElement } = renderer;
     /**
      * Note: if the upgradable constructor does not expect, or throw when we new it
@@ -328,7 +329,10 @@ function mountCustomElement(
     // compiler may generate tagnames with uppercase letters so - for backwards
     // compatibility, we lower case the tagname here.
     const normalizedTagname = sel.toLowerCase();
-    const elm = createCustomElement(normalizedTagname, upgradeCallback);
+    const useNativeLifecycle = shouldUseNativeCustomElementLifecycle(
+        ctor as LightningElementConstructor
+    );
+    const elm = createCustomElement(normalizedTagname, upgradeCallback, useNativeLifecycle);
 
     vnode.elm = elm;
     vnode.vm = vm;
@@ -345,7 +349,7 @@ function mountCustomElement(
 
     if (vm) {
         if (process.env.IS_BROWSER) {
-            if (!lwcRuntimeFlags.ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE) {
+            if (!useNativeLifecycle) {
                 if (process.env.NODE_ENV !== 'production') {
                     // With synthetic lifecycle callbacks, it's possible for elements to be removed without the engine
                     // noticing it (e.g. `appendChild` the same host element twice). This test ensures we don't regress.

diff --git a/packages/@lwc/engine-core/src/framework/utils.ts b/packages/@lwc/engine-core/src/framework/utils.ts
@@ -4,8 +4,19 @@
  * SPDX-License-Identifier: MIT
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
  */
-import { ArrayPush, create, isArray, isFunction, keys, seal } from '@lwc/shared';
+import {
+    ArrayPush,
+    create,
+    isArray,
+    isFunction,
+    keys,
+    seal,
+    isAPIFeatureEnabled,
+    APIFeature,
+} from '@lwc/shared';
 import { StylesheetFactory, TemplateStylesheetFactories } from './stylesheet';
+import { getComponentAPIVersion } from './component';
+import { LightningElementConstructor } from './base-lightning-element';
 
 type Callback = () => void;
 
@@ -54,6 +65,17 @@ export function guid(): string {
     return s4() + s4() + '-' + s4() + '-' + s4() + '-' + s4() + '-' + s4() + s4() + s4();
 }
 
+export function shouldUseNativeCustomElementLifecycle(ctor: LightningElementConstructor) {
+    if (lwcRuntimeFlags.DISABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE) {
+        // temporary "kill switch"
+        return false;
+    }
+
+    const apiVersion = getComponentAPIVersion(ctor);
+
+    return isAPIFeatureEnabled(APIFeature.ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE, apiVersion);
+}
+
 // Borrowed from Vue template compiler.
 // https://github.com/vuejs/vue/blob/531371b818b0e31a989a06df43789728f23dc4e8/src/platforms/web/util/style.js#L5-L16
 const DECLARATION_DELIMITER = /;(?![^(]*\))/g;

diff --git a/packages/@lwc/engine-core/src/framework/vm.ts b/packages/@lwc/engine-core/src/framework/vm.ts
@@ -32,7 +32,13 @@ import {
     getTemplateReactiveObserver,
     getComponentAPIVersion,
 } from './component';
-import { addCallbackToNextTick, EmptyArray, EmptyObject, flattenStylesheets } from './utils';
+import {
+    addCallbackToNextTick,
+    EmptyArray,
+    EmptyObject,
+    flattenStylesheets,
+    shouldUseNativeCustomElementLifecycle,
+} from './utils';
 import { invokeComponentCallback, invokeComponentConstructor } from './invoker';
 import { Template } from './template';
 import { ComponentDef, getComponentInternalDef } from './def';
@@ -280,7 +286,11 @@ function resetComponentStateWhenRemoved(vm: VM) {
 // old vnode.children is removed from the DOM.
 export function removeVM(vm: VM) {
     if (process.env.NODE_ENV !== 'production') {
-        if (!lwcRuntimeFlags.ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE) {
+        if (
+            !shouldUseNativeCustomElementLifecycle(
+                vm.component.constructor as LightningElementConstructor
+            )
+        ) {
             // With native lifecycle, we cannot be certain that connectedCallback was called before a component
             // was removed from the VDOM. If the component is disconnected, then connectedCallback will not fire
             // in native mode, although it will fire in synthetic mode due to appendChild triggering it.
@@ -691,7 +701,9 @@ export function runConnectedCallback(vm: VM) {
     // we're in dev mode. This is to detect a particular issue with synthetic lifecycle.
     if (
         process.env.IS_BROWSER &&
-        !lwcRuntimeFlags.ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE &&
+        !shouldUseNativeCustomElementLifecycle(
+            vm.component.constructor as LightningElementConstructor
+        ) &&
         (process.env.NODE_ENV !== 'production' || isReportingEnabled())
     ) {
         if (!vm.renderer.isConnected(vm.elm)) {

diff --git a/packages/@lwc/engine-dom/src/apis/create-element.ts b/packages/@lwc/engine-dom/src/apis/create-element.ts
@@ -13,12 +13,15 @@ import {
     isUndefined,
     toString,
     StringToLowerCase,
+    isAPIFeatureEnabled,
+    APIFeature,
 } from '@lwc/shared';
 import {
     createVM,
     connectRootElement,
     disconnectRootElement,
     LightningElement,
+    getComponentAPIVersion,
 } from '@lwc/engine-core';
 import { renderer } from '../renderer';
 
@@ -45,7 +48,14 @@ function callNodeSlot(node: Node, slot: WeakMap<any, NodeSlotCallback>): Node {
     return node; // for convenience
 }
 
-if (!lwcRuntimeFlags.ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE) {
+let monkeyPatched = false;
+
+function monkeyPatchDomAPIs() {
+    if (monkeyPatched) {
+        // don't double-patch
+        return;
+    }
+    monkeyPatched = true;
     // Monkey patching Node methods to be able to detect the insertions and removal of root elements
     // created via createElement.
     const { appendChild, insertBefore, removeChild, replaceChild } = _Node.prototype;
@@ -111,6 +121,13 @@ export function createElement(
     // the following line guarantees that this does not leaks beyond this point.
     const tagName = StringToLowerCase.call(sel);
 
+    const apiVersion = getComponentAPIVersion(Ctor);
+
+    const useNativeCustomElementLifecycle =
+        // temporary "kill switch"
+        !lwcRuntimeFlags.DISABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE &&
+        isAPIFeatureEnabled(APIFeature.ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE, apiVersion);
+
     // the custom element from the registry is expecting an upgrade callback
     /**
      * Note: if the upgradable constructor does not expect, or throw when we new it
@@ -124,12 +141,14 @@ export function createElement(
             mode: options.mode !== 'closed' ? 'open' : 'closed',
             owner: null,
         });
-        if (!lwcRuntimeFlags.ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE) {
+        if (!useNativeCustomElementLifecycle) {
+            // Monkey-patch on-demand, because if there are no components on the page using an old API
+            // version, then we don't want to monkey patch at all
+            monkeyPatchDomAPIs();
             ConnectingSlot.set(elm, connectRootElement);
             DisconnectingSlot.set(elm, disconnectRootElement);
         }
     };
 
-    const element = createCustomElement(tagName, upgradeCallback);
-    return element;
+    return createCustomElement(tagName, upgradeCallback, useNativeCustomElementLifecycle);
 }
diff --git a/packages/@lwc/engine-dom/src/custom-elements/create-custom-element.ts b/packages/@lwc/engine-dom/src/custom-elements/create-custom-element.ts
@@ -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 { isUndefined, entries } from '@lwc/shared';
+import { isUndefined, entries, isTrue } from '@lwc/shared';
 import {
     LifecycleCallback,
     connectRootElement,
@@ -14,20 +14,20 @@ import {
     runFormResetCallback,
     runFormStateRestoreCallback,
 } from '@lwc/engine-core';
+
+const LIFECYCLE_CALLBACKS = {
+    connectedCallback: connectRootElement,
+    disconnectedCallback: disconnectRootElement,
+    formAssociatedCallback: runFormAssociatedCallback,
+    formDisabledCallback: runFormDisabledCallback,
+    formResetCallback: runFormResetCallback,
+    formStateRestoreCallback: runFormStateRestoreCallback,
+};
+
 const cachedConstructors = new Map<string, CustomElementConstructor>();
-const elementsUpgradedOutsideLWC = new WeakSet<HTMLElement>();
-let elementBeingUpgradedByLWC = false;
+const nativeLifecycleElementsToUpgradedByLWC = new WeakMap<HTMLElement, boolean>();
 
-const lifecycleCallbacks = lwcRuntimeFlags.ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE
-    ? {
-          connectedCallback: connectRootElement,
-          disconnectedCallback: disconnectRootElement,
-          formAssociatedCallback: runFormAssociatedCallback,
-          formDisabledCallback: runFormDisabledCallback,
-          formResetCallback: runFormResetCallback,
-          formStateRestoreCallback: runFormStateRestoreCallback,
-      }
-    : undefined;
+let elementBeingUpgradedByLWC = false;
 
 // Creates a constructor that is intended to be used directly as a custom element, except that the upgradeCallback is
 // passed in to the constructor so LWC can reuse the same custom element constructor for multiple components.
@@ -39,34 +39,34 @@ const createUpgradableConstructor = () => {
     class UpgradableConstructor extends HTMLElement {
         static formAssociated = true;
 
-        constructor(upgradeCallback: LifecycleCallback) {
+        constructor(upgradeCallback: LifecycleCallback, useNativeLifecycle: boolean) {
             super();
+
+            if (useNativeLifecycle) {
+                // When in native lifecycle mode, we need to keep track of instances that were created outside LWC
+                // (i.e. not created by `lwc.createElement()`). If the element uses synthetic lifecycle, then we don't
+                // need to track this.
+                nativeLifecycleElementsToUpgradedByLWC.set(this, elementBeingUpgradedByLWC);
+            }
+
             // If the element is not created using lwc.createElement(), e.g. `document.createElement('x-foo')`,
-            // then elementBeingUpgraded will be false
+            // then elementBeingUpgradedByLWC will be false
             if (elementBeingUpgradedByLWC) {
                 upgradeCallback(this);
-            } else if (!isUndefined(lifecycleCallbacks)) {
-                // If this element has any lifecycle callbacks, then we need to keep track of
-                // instances that were created outside LWC (i.e. not created by `lwc.createElement()`).
-                // If the element has no connected or disconnected callbacks, then we don't need to track this.
-                elementsUpgradedOutsideLWC.add(this);
-
-                // TODO [#2970]: LWC elements cannot be upgraded via new Ctor()
-                // Do we want to support this? Throw an error? Currently for backwards compat it's a no-op.
             }
+            // TODO [#2970]: LWC elements cannot be upgraded via new Ctor()
+            // Do we want to support this? Throw an error? Currently for backwards compat it's a no-op.
         }
     }
 
-    // Do not unnecessarily add a connectedCallback/disconnectedCallback/etc., as it introduces perf overhead
-    // See: https://github.com/salesforce/lwc/pull/3162#issuecomment-1311851174
-    if (!isUndefined(lifecycleCallbacks)) {
-        for (const [propName, callback] of entries(lifecycleCallbacks)) {
-            (UpgradableConstructor.prototype as any)[propName] = function () {
-                if (!elementsUpgradedOutsideLWC.has(this)) {
-                    callback(this);
-                }
-            };
-        }
+    for (const [propName, callback] of entries(LIFECYCLE_CALLBACKS)) {
+        (UpgradableConstructor.prototype as any)[propName] = function () {
+            // If the element is in the WeakMap (i.e. it's marked as native lifecycle), and if it was upgraded by LWC,
+            // then it can use native lifecycle
+            if (isTrue(nativeLifecycleElementsToUpgradedByLWC.get(this))) {
+                callback(this);
+            }
+        };
     }
 
     return UpgradableConstructor;
@@ -88,12 +88,16 @@ export function getUpgradableConstructor(tagName: string) {
     return UpgradableConstructor;
 }
 
-export const createCustomElement = (tagName: string, upgradeCallback: LifecycleCallback) => {
+export const createCustomElement = (
+    tagName: string,
+    upgradeCallback: LifecycleCallback,
+    useNativeLifecycle: boolean
+) => {
     const UpgradableConstructor = getUpgradableConstructor(tagName);
 
     elementBeingUpgradedByLWC = true;
     try {
-        return new UpgradableConstructor(upgradeCallback);
+        return new UpgradableConstructor(upgradeCallback, useNativeLifecycle);
     } finally {
         elementBeingUpgradedByLWC = false;
     }

diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts b/packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts
@@ -9,12 +9,9 @@ import fs from 'fs';
 import path from 'path';
 
 import { rollup, RollupLog } from 'rollup';
-// @ts-ignore
 import lwcRollupPlugin from '@lwc/rollup-plugin';
 import { isVoidElement, HTML_NAMESPACE } from '@lwc/shared';
 import { testFixtureDir } from '@lwc/jest-utils-lwc-internals';
-import { setFeatureFlagForTest } from '../index';
-import type { FeatureFlagMap } from '@lwc/features';
 import type * as lwc from '../index';
 
 interface FixtureModule {
@@ -206,26 +203,6 @@ function testFixtures() {
     );
 }
 
-// Run the fixtures with both synthetic and native custom element lifecycle.
-// The expectation is that the fixtures will be exactly the same for both.
 describe('fixtures', () => {
-    describe('synthetic custom element lifecycle', () => {
-        testFixtures();
-    });
-
-    function testWithFeatureFlagEnabled(flagName: keyof FeatureFlagMap) {
-        beforeEach(() => {
-            setFeatureFlagForTest(flagName, true);
-        });
-
-        afterEach(() => {
-            setFeatureFlagForTest(flagName, false);
-        });
-
-        testFixtures();
-    }
-
-    describe('native custom element lifecycle', () => {
-        testWithFeatureFlagEnabled('ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE');
-    });
+    testFixtures();
 });
diff --git a/packages/@lwc/engine-server/src/renderer.ts b/packages/@lwc/engine-server/src/renderer.ts
@@ -366,7 +366,12 @@ function getUpgradableElement(
     return ctor;
 }
 
-function createCustomElement(tagName: string, upgradeCallback: LifecycleCallback): HostElement {
+// Note that SSR does not have any concept of native vs synthetic custom element lifecycle
+function createCustomElement(
+    tagName: string,
+    upgradeCallback: LifecycleCallback,
+    _useNativeLifecycle: boolean
+): HostElement {
     const UpgradableConstructor = getUpgradableElement(tagName);
     return new (UpgradableConstructor as any)(upgradeCallback);
 }

diff --git a/packages/@lwc/features/src/index.ts b/packages/@lwc/features/src/index.ts
@@ -13,7 +13,7 @@ const features: FeatureFlagMap = {
     PLACEHOLDER_TEST_FLAG: null,
     ENABLE_FORCE_NATIVE_SHADOW_MODE_FOR_TEST: null,
     ENABLE_MIXED_SHADOW_MODE: null,
-    ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE: null,
+    DISABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE: null,
     ENABLE_WIRE_SYNC_EMIT: null,
     DISABLE_LIGHT_DOM_UNSCOPED_CSS: null,
     ENABLE_FROZEN_TEMPLATE: null,