From a809d7b5e1e4a8c6af158cb36d421618e3b6c402 Mon Sep 17 00:00:00 2001
From: Joren Broekema <joren.broekema@ing.com>
Date: Tue, 16 Mar 2021 10:37:33 +0100
Subject: [PATCH] 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                 |  5 +++
 .../form-core/src/utils/SyncUpdatableMixin.js | 15 +++++---
 .../test/utils/SyncUpdatableMixin.test.js     | 36 +++++++++++++++++++
 .../test/form-integrations.test.js            |  8 ++---
 .../test/helpers/umbrella-form.js             |  4 +--
 5 files changed, 58 insertions(+), 10 deletions(-)
 create mode 100644 .changeset/sweet-terms-act.md

diff --git a/.changeset/sweet-terms-act.md b/.changeset/sweet-terms-act.md
new file mode 100644
index 000000000..c7365946c
--- /dev/null
+++ b/.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.
diff --git a/packages/form-core/src/utils/SyncUpdatableMixin.js b/packages/form-core/src/utils/SyncUpdatableMixin.js
index eb8b7d12b..f0ccb2226 100644
--- a/packages/form-core/src/utils/SyncUpdatableMixin.js
+++ b/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);
diff --git a/packages/form-core/test/utils/SyncUpdatableMixin.test.js b/packages/form-core/test/utils/SyncUpdatableMixin.test.js
index 28f7c7488..b9fd6f0f6 100644
--- a/packages/form-core/test/utils/SyncUpdatableMixin.test.js
+++ b/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;
+  });
 });
diff --git a/packages/form-integrations/test/form-integrations.test.js b/packages/form-integrations/test/form-integrations.test.js
index 6d28f3bd6..a9b024778 100644
--- a/packages/form-integrations/test/form-integrations.test.js
+++ b/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[]': [[]],
     });
   });
 });
diff --git a/packages/form-integrations/test/helpers/umbrella-form.js b/packages/form-integrations/test/helpers/umbrella-form.js
index 578575026..fb60807ff 100644
--- a/packages/form-integrations/test/helpers/umbrella-form.js
+++ b/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>