[reactive-element] Fix a bug in change detection with decorated standard private accessors (#4999)

Author: justinfagnani

.changeset/proud-pets-draw.md

@@ -0,0 +1,7 @@
+---
+'@lit/reactive-element': patch
+'lit': patch
+'lit-element': patch
+---
+
+Fix a bug in change detection with decorated standard private accessors.

packages/reactive-element/package.json

@@ -251,6 +251,7 @@
       ],
       "files": [
         "src/**/*.ts",
+        "!src/test/decorators-modern/decorators-standard/**/*.ts",
         "tsconfig.json",
         "tsconfig.polyfill-support.json"
       ],
@@ -274,7 +275,8 @@
         "decorators/*.d.ts{,.map}",
         "legacy-decorators/*.d.ts{,.map}",
         "std-decorators/*.d.ts{,.map}"
-      ]
+      ],
+      "clean": "if-file-deleted"
     },
     "build:ts:std-decorators-tests": {
       "#comment": "This is a separate script from build:ts because it needs a tsconfig without experimentalDecorators.",
@@ -285,6 +287,7 @@
       ],
       "files": [
         "src/test/decorators-modern/**/*.ts",
+        "src/test/decorators-standard/**/*.ts",
         "tsconfig.std-decorators-tests.json"
       ],
       "output": [
@@ -318,7 +321,7 @@
       ]
     },
     "build:babel": {
-      "command": "babel --extensions .ts src/test/decorators-modern --out-dir development/test/decorators-babel",
+      "command": "babel --extensions .ts src/test/decorators-modern/*_test.ts --out-dir development/test/decorators-babel",
       "files": [
         ".babelrc",
         "src/test/decorators-modern/**/*.ts"
@@ -330,7 +333,8 @@
     "build:esbuild": {
       "command": "esbuild src/test/decorators-modern/**/*.ts --outdir=development/test/decorators-esbuild --target=es2022",
       "files": [
-        "src/test/decorators-modern/**/*.ts"
+        "src/test/decorators-modern/**/*.ts",
+        "!src/test/decorators-modern/decorators-standard/**/*.ts"
       ],
       "output": [
         "development/test/decorators-esbuild"

packages/reactive-element/src/decorators/property.ts

@@ -150,7 +150,7 @@ export const standardProperty = <C extends Interface<ReactiveElement>, V>(
           this as unknown as C,
           v
         );
-        this.requestUpdate(name, oldValue, options);
+        this.requestUpdate(name, oldValue, options, true, v);
       },
       init(this: ReactiveElement, v: V): V {
         if (v !== undefined) {
@@ -164,7 +164,7 @@ export const standardProperty = <C extends Interface<ReactiveElement>, V>(
     return function (this: ReactiveElement, value: V) {
       const oldValue = this[name as keyof ReactiveElement];
       (target as (value: V) => void).call(this, value);
-      this.requestUpdate(name, oldValue, options);
+      this.requestUpdate(name, oldValue, options, true, value);
     } as unknown as (this: C, value: V) => void;
   }
   throw new Error(`Unsupported decorator location: ${kind}`);

packages/reactive-element/src/reactive-element.ts

@@ -1270,12 +1270,20 @@ export abstract class ReactiveElement
    * @param oldValue old value of requesting property
    * @param options property options to use instead of the previously
    *     configured options
+   * @param useNewValue if true, the newValue argument is used instead of
+   *     reading the property value. This is important to use if the reactive
+   *     property is a standard private accessor, as opposed to a plain
+   *     property, since private members can't be dynamically read by name.
+   * @param newValue the new value of the property. This is only used if
+   *     `useNewValue` is true.
    * @category updates
    */
   requestUpdate(
     name?: PropertyKey,
     oldValue?: unknown,
-    options?: PropertyDeclaration
+    options?: PropertyDeclaration,
+    useNewValue = false,
+    newValue?: unknown
   ): void {
     // If we have a property key, perform property update steps.
     if (name !== undefined) {
@@ -1286,7 +1294,9 @@ export abstract class ReactiveElement
         );
       }
       const ctor = this.constructor as typeof ReactiveElement;
-      const newValue = this[name as keyof this];
+      if (useNewValue === false) {
+        newValue = this[name as keyof this];
+      }
       options ??= ctor.getPropertyOptions(name);
       const changed =
         (options.hasChanged ?? notEqual)(newValue, oldValue) ||

packages/reactive-element/src/test/decorators-modern/decorators-standard/README.md

@@ -0,0 +1,3 @@
+This folder is excluded from the plain tsconfig that uses
+`experimentalDecorators: true` because it includes decorators on private class
+members which aren't allowed by experimental decorators.

packages/reactive-element/src/test/decorators-modern/decorators-standard/state_test.ts

@@ -0,0 +1,112 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+import {ReactiveElement, PropertyValues} from '@lit/reactive-element';
+import {state} from '@lit/reactive-element/decorators/state.js';
+import {generateElementName} from '../../test-helpers.js';
+import {assert} from 'chai';
+
+suite('@state', () => {
+  let container: HTMLElement;
+  let el: E;
+
+  const hasChanged = (value: any, old: any) => old === undefined || value > old;
+
+  class E extends ReactiveElement {
+    @state()
+    accessor #prop: string | undefined = undefined;
+
+    get prop() {
+      return this.#prop;
+    }
+
+    set prop(v) {
+      this.#prop = v;
+    }
+
+    @state({hasChanged})
+    accessor #hasChangedProp = 10;
+
+    get hasChangedProp() {
+      return this.#hasChangedProp;
+    }
+
+    set hasChangedProp(v) {
+      this.#hasChangedProp = v;
+    }
+
+    #setterValue: string | undefined = undefined;
+
+    @state()
+    set #setter(v: string | undefined) {
+      this.#setterValue = v;
+    }
+    get #setter() {
+      return this.#setterValue;
+    }
+
+    set setter(v: string | undefined) {
+      this.#setter = v;
+    }
+    get setter() {
+      return this.#setter;
+    }
+
+    updateCount = 0;
+
+    override update(changed: PropertyValues) {
+      this.updateCount++;
+      super.update(changed);
+    }
+  }
+  customElements.define(generateElementName(), E);
+
+  setup(async () => {
+    container = document.createElement('div');
+    container.id = 'test-container';
+    document.body.appendChild(container);
+    el = new E();
+    container.appendChild(el);
+    await el.updateComplete;
+  });
+
+  teardown(() => {
+    if (container !== undefined) {
+      container.parentElement!.removeChild(container);
+      (container as any) = undefined;
+    }
+  });
+
+  test('triggers accessor update', async () => {
+    assert.equal(el.updateCount, 1);
+    el.prop = 'change';
+    await el.updateComplete;
+    // If we don't read the new value of private accessors correctly, this will
+    // fail.
+    assert.equal(el.updateCount, 2);
+  });
+
+  test('uses hasChanged', async () => {
+    assert.equal(el.updateCount, 1);
+    el.hasChangedProp = 100;
+    await el.updateComplete;
+    // If we don't read the new value of private accessors correctly, this will
+    // fail.
+    assert.equal(el.updateCount, 2);
+    el.hasChangedProp = 0;
+    await el.updateComplete;
+    assert.equal(el.updateCount, 2);
+  });
+
+  test('triggers setter update', async () => {
+    assert.equal(el.updateCount, 1);
+    el.setter = 'change';
+    await el.updateComplete;
+    // If we don't read the new value of private accessors correctly, this will
+    // fail.
+    assert.equal(el.updateCount, 2);
+  });
+});

packages/reactive-element/tsconfig.json

@@ -28,6 +28,9 @@
     "../../node_modules/@webcomponents/webcomponentsjs/webcomponents-bundle.d.ts",
     "src/**/*.ts"
   ],
-  "exclude": ["src/polyfill-support.ts"],
+  "exclude": [
+    "src/polyfill-support.ts",
+    "src/test/decorators-modern/decorators-standard/**/*.ts"
+  ],
   "references": [{"path": "./tsconfig.polyfill-support.json"}]
 }

scripts/check-size.js

@@ -9,7 +9,7 @@ import * as fs from 'fs';
 // it's likely that we'll ask you to investigate ways to reduce the size.
 //
 // In either case, update the size here and push a new commit to your PR.
-const expectedLitCoreSize = 15717;
+const expectedLitCoreSize = 15734;
 const expectedLitHtmlSize = 7309;
 
 const litCoreSrc = fs.readFileSync('packages/lit/lit-core.min.js', 'utf8');