fix: make sync updatable mixin work with re-connecting to DOM

Co-authored-by: Thijs Louisse <Thijs.Louisse@ing.com>

.changeset/sweet-terms-act.md

@@ -0,0 +1,5 @@
+---
+'@lion/form-core': patch
+---
+
+Make sync updatable mixin work for elements that get re-connected to DOM e.g. through appendChild. Needed for integration with global overlays.

packages/form-core/src/utils/SyncUpdatableMixin.js

@@ -34,13 +34,21 @@ const SyncUpdatableMixinImplementation = superclass =>
       this.__SyncUpdatableNamespace = {};
     }
 
-    /** @param {import('@lion/core').PropertyValues } changedProperties */
+    /**
+     * Empty pending queue in order to guarantee order independence
+     *
+     * @param {import('lit-element').PropertyValues } changedProperties
+     */
     firstUpdated(changedProperties) {
       super.firstUpdated(changedProperties);
-      this.__SyncUpdatableNamespace.connected = true;
       this.__syncUpdatableInitialize();
     }
 
+    connectedCallback() {
+      super.connectedCallback();
+      this.__SyncUpdatableNamespace.connected = true;
+    }
+
     disconnectedCallback() {
       super.disconnectedCallback();
       this.__SyncUpdatableNamespace.connected = false;
@@ -89,9 +97,8 @@ const SyncUpdatableMixinImplementation = superclass =>
 
       const ctor = /** @type {typeof SyncUpdatableMixin & typeof import('../../types/utils/SyncUpdatableMixinTypes').SyncUpdatableHost} */ (this
         .constructor);
-
       // Before connectedCallback: queue
-      if (!ns.connected) {
+      if (!ns.initialized) {
         ns.queue = ns.queue || new Set();
         // Makes sure that we only initialize one time, with most up to date value
         ns.queue.add(name);

packages/form-core/test/utils/SyncUpdatableMixin.test.js

@@ -300,4 +300,40 @@ describe('SyncUpdatableMixin', () => {
       expect(spy.callCount).to.equal(3);
     });
   });
+
+  it('reinitializes when the element gets reconnected to DOM', async () => {
+    const container = await fixture(html`<div></div>`);
+    class UpdatableImplementation extends SyncUpdatableMixin(LitElement) {
+      static get properties() {
+        return {
+          prop: { attribute: false },
+        };
+      }
+
+      constructor() {
+        super();
+        this.prop = '';
+      }
+    }
+    const tagString = defineCE(UpdatableImplementation);
+    const tag = unsafeStatic(tagString);
+    const el = /** @type {UpdatableImplementation} */ (fixtureSync(html`<${tag}></${tag}>`));
+    const ns = el.__SyncUpdatableNamespace;
+    const updateSyncSpy = sinon.spy(el, 'updateSync');
+
+    expect(ns.connected).to.be.true;
+    expect(ns.initialized).to.be.undefined;
+    await el.updateComplete;
+    expect(ns.initialized).to.be.true;
+    el.parentElement?.removeChild(el);
+    expect(ns.connected).to.be.false;
+    container.appendChild(el);
+    expect(ns.connected).to.be.true;
+
+    // change a prop to cause rerender
+    expect(updateSyncSpy.calledWith('prop', undefined)).to.be.true;
+    el.prop = 'a';
+    await el.updateComplete;
+    expect(updateSyncSpy.calledWith('prop', '')).to.be.true;
+  });
 });

packages/form-integrations/test/form-integrations.test.js

@@ -13,7 +13,7 @@ describe('Form Integrations', () => {
     const formEl = el._lionFormNode;
     expect(formEl.serializedValue).to.eql({
       bio: '',
-      checkers: ['foo', 'bar'],
+      'checkers[]': [['foo', 'bar']],
       comments: '',
       date: '2000-12-12',
       datepicker: '2020-12-12',
@@ -28,7 +28,7 @@ describe('Form Integrations', () => {
       lyrics: '1',
       money: '',
       range: 2.3,
-      terms: [],
+      'terms[]': [[]],
     });
   });
 
@@ -38,7 +38,7 @@ describe('Form Integrations', () => {
     const formEl = el._lionFormNode;
     expect(formEl.formattedValue).to.eql({
       bio: '',
-      checkers: ['foo', 'bar'],
+      'checkers[]': [['foo', 'bar']],
       comments: '',
       date: '12/12/2000',
       datepicker: '12/12/2020',
@@ -53,7 +53,7 @@ describe('Form Integrations', () => {
       lyrics: '1',
       money: '',
       range: 2.3,
-      terms: [],
+      'terms[]': [[]],
     });
   });
 });

packages/form-integrations/test/helpers/umbrella-form.js

@@ -62,7 +62,7 @@ export class UmbrellaForm extends LitElement {
           <lion-input-email name="email" label="Email"></lion-input-email>
           <lion-checkbox-group
             label="What do you like?"
-            name="checkers"
+            name="checkers[]"
             .validators="${[new Required()]}"
           >
             <lion-checkbox .choiceValue=${'foo'} checked label="I like foo"></lion-checkbox>
@@ -102,7 +102,7 @@ export class UmbrellaForm extends LitElement {
             label="Input range"
           >
           </lion-input-range>
-          <lion-checkbox-group name="terms" .validators="${[new Required()]}">
+          <lion-checkbox-group name="terms[]" .validators="${[new Required()]}">
             <lion-checkbox label="I blindly accept all terms and conditions"></lion-checkbox>
           </lion-checkbox-group>
           <lion-textarea name="comments" label="Comments"></lion-textarea>