From 6361a4b4a589465cf6836c8454ed8ca4521d7b4d Mon Sep 17 00:00:00 2001
From: Francesco Benedetto <francesco.benedetto263@gmail.com>
Date: Fri, 5 Aug 2022 20:16:31 +0200
Subject: [PATCH] [reactive-element] Bind this to custom attribute converter
 methods (#3120)

---
 .changeset/old-tables-own.md                  |  6 ++
 .../reactive-element/src/reactive-element.ts  | 30 ++++----
 .../src/test/reactive-element_test.ts         | 73 +++++++++++++++++++
 3 files changed, 96 insertions(+), 13 deletions(-)
 create mode 100644 .changeset/old-tables-own.md

diff --git a/.changeset/old-tables-own.md b/.changeset/old-tables-own.md
new file mode 100644
index 0000000000..6905c92bdf
--- /dev/null
+++ b/.changeset/old-tables-own.md
@@ -0,0 +1,6 @@
+---
+'@lit/reactive-element': patch
+'lit': patch
+---
+
+Bind `this` to custom attribute converter methods
diff --git a/packages/reactive-element/src/reactive-element.ts b/packages/reactive-element/src/reactive-element.ts
index 15dfe38e94..3cdb991c6b 100644
--- a/packages/reactive-element/src/reactive-element.ts
+++ b/packages/reactive-element/src/reactive-element.ts
@@ -1083,10 +1083,12 @@ export abstract class ReactiveElement
       this.constructor as typeof ReactiveElement
     ).__attributeNameForProperty(name, options);
     if (attr !== undefined && options.reflect === true) {
-      const toAttribute =
-        (options.converter as ComplexAttributeConverter)?.toAttribute ??
-        defaultConverter.toAttribute;
-      const attrValue = toAttribute!(value, options.type);
+      const converter =
+        (options.converter as ComplexAttributeConverter)?.toAttribute !==
+        undefined
+          ? (options.converter as ComplexAttributeConverter)
+          : defaultConverter;
+      const attrValue = converter.toAttribute!(value, options.type);
       if (
         DEV_MODE &&
         (this.constructor as typeof ReactiveElement).enabledWarnings!.indexOf(
@@ -1131,17 +1133,19 @@ export abstract class ReactiveElement
     // if it was just set because the attribute changed.
     if (propName !== undefined && this.__reflectingProperty !== propName) {
       const options = ctor.getPropertyOptions(propName);
-      const converter = options.converter;
-      const fromAttribute =
-        (converter as ComplexAttributeConverter)?.fromAttribute ??
-        (typeof converter === 'function'
-          ? (converter as (value: string | null, type?: unknown) => unknown)
-          : null) ??
-        defaultConverter.fromAttribute;
+      const converter =
+        typeof options.converter === 'function'
+          ? {fromAttribute: options.converter}
+          : options.converter?.fromAttribute !== undefined
+          ? options.converter
+          : defaultConverter;
       // mark state reflecting
       this.__reflectingProperty = propName;
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      this[propName as keyof this] = fromAttribute!(value, options.type) as any;
+      this[propName as keyof this] = converter.fromAttribute!(
+        value,
+        options.type
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      ) as any;
       // mark state not reflecting
       this.__reflectingProperty = null;
     }
diff --git a/packages/reactive-element/src/test/reactive-element_test.ts b/packages/reactive-element/src/test/reactive-element_test.ts
index 468f46341d..427a40c8dc 100644
--- a/packages/reactive-element/src/test/reactive-element_test.ts
+++ b/packages/reactive-element/src/test/reactive-element_test.ts
@@ -302,6 +302,79 @@ suite('ReactiveElement', () => {
     assert.equal(el.getAttribute('foo'), 'toAttribute: FooType');
   });
 
+  test('property option `converter` can use a class instance', async () => {
+    class IntegerAttributeConverter
+      implements ComplexAttributeConverter<Number>
+    {
+      private _defaultValue: Number;
+
+      constructor(defaultValue: Number) {
+        this._defaultValue = defaultValue;
+      }
+
+      toAttribute(value: Number, _type?: unknown): unknown {
+        if (!value) {
+          return this._defaultValue;
+        }
+        return `${value}`;
+      }
+
+      fromAttribute(value: string | null, _type?: unknown): Number {
+        if (!value) {
+          return this._defaultValue;
+        }
+
+        const parsedValue = Number.parseInt(value, 10);
+        if (isNaN(parsedValue)) {
+          return this._defaultValue;
+        }
+        return parsedValue;
+      }
+    }
+
+    const defaultIntAttrConverterVal = 1;
+
+    class E extends ReactiveElement {
+      static override get properties() {
+        return {
+          num: {
+            type: Number,
+            converter: new IntegerAttributeConverter(
+              defaultIntAttrConverterVal
+            ),
+            reflect: true,
+          },
+        };
+      }
+
+      num?: number;
+    }
+
+    customElements.define(generateElementName(), E);
+    const el = new E();
+    container.appendChild(el);
+    await el.updateComplete;
+
+    assert.equal(el.getAttribute('num'), null);
+    assert.equal(el.num, undefined);
+
+    el.setAttribute('num', 'notANumber');
+    await el.updateComplete;
+    assert.equal(el.num, defaultIntAttrConverterVal);
+
+    el.num = 10;
+    await el.updateComplete;
+    assert.equal(el.getAttribute('num'), '10');
+
+    el.setAttribute('num', '5');
+    await el.updateComplete;
+    assert.equal(el.num, 5);
+
+    el.num = undefined;
+    await el.updateComplete;
+    assert.equal(el.getAttribute('num'), `${defaultIntAttrConverterVal}`);
+  });
+
   test('property/attribute values when attributes removed', async () => {
     class E extends ReactiveElement {
       static override get properties() {