From ca9ee25d011656b994deff01112dc6538850fc25 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= <sissbruecker@vaadin.com>
Date: Tue, 11 Oct 2022 12:33:02 +0200
Subject: [PATCH] fix: avoid triggering connected callback from disconnected
 callback (#4718)

When moving an element that extends from `DirMixin` in the DOM, the `DirMixin.disconnectedCallback` can trigger the element's `connectedCallback` before the `disconnectedCallback` has finished (currently reproducable in Chrome, FF). That can cause elements to re-run their initialization logic before their cleanup logic is finished. For example in case of the `AppLayout`, this results in the `connectedCallback` adding several event listeners again, after which the remaining logic in
`disconnectedCallback` removes them again, leaving the element in a state where the listeners are not registered. See https://github.com/vaadin/web-components/issues/4682#issuecomment-1273226985.

This change removes the `removeAttribute` call from `DirMixin.disconnectedCallback` that causes the premature reconnect, and instead introduces a flag that ensures that the element will subscribe to global dir changes again on reconnect. This also fixes a minor bug where the custom dir of an element was removed on disconnect (see test case).

Fixes: #4682
---
 packages/component-base/src/dir-mixin.js      | 24 +++++++++++--------
 .../component-base/test/dir-mixin.test.js     |  9 +++++++
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/packages/component-base/src/dir-mixin.js b/packages/component-base/src/dir-mixin.js
index 3f3667ed9b6..342f86a1db2 100644
--- a/packages/component-base/src/dir-mixin.js
+++ b/packages/component-base/src/dir-mixin.js
@@ -9,6 +9,7 @@ import { DirHelper } from './dir-helper.js';
  * Array of Vaadin custom element classes that have been subscribed to the dir changes.
  */
 const directionSubscribers = [];
+
 function directionUpdater() {
   const documentDir = getDocumentDir();
   directionSubscribers.forEach((element) => {
@@ -74,7 +75,7 @@ export const DirMixin = (superClass) =>
     connectedCallback() {
       super.connectedCallback();
 
-      if (!this.hasAttribute('dir')) {
+      if (!this.hasAttribute('dir') || this.__restoreSubscription) {
         this.__subscribe();
         alignDirs(this, getDocumentDir(), null);
       }
@@ -100,15 +101,15 @@ export const DirMixin = (superClass) =>
         this.__subscribe();
         alignDirs(this, documentDir, newValue);
       } else if (newDiffValue) {
-        this.__subscribe(false);
+        this.__unsubscribe();
       }
     }
 
     /** @protected */
     disconnectedCallback() {
       super.disconnectedCallback();
-      this.__subscribe(false);
-      this.removeAttribute('dir');
+      this.__restoreSubscription = directionSubscribers.includes(this);
+      this.__unsubscribe();
     }
 
     /** @protected */
@@ -133,12 +134,15 @@ export const DirMixin = (superClass) =>
     }
 
     /** @private */
-    __subscribe(push = true) {
-      if (push) {
-        if (!directionSubscribers.includes(this)) {
-          directionSubscribers.push(this);
-        }
-      } else if (directionSubscribers.includes(this)) {
+    __subscribe() {
+      if (!directionSubscribers.includes(this)) {
+        directionSubscribers.push(this);
+      }
+    }
+
+    /** @private */
+    __unsubscribe() {
+      if (directionSubscribers.includes(this)) {
         directionSubscribers.splice(directionSubscribers.indexOf(this), 1);
       }
     }
diff --git a/packages/component-base/test/dir-mixin.test.js b/packages/component-base/test/dir-mixin.test.js
index e7dae2225ac..e925e3bf322 100644
--- a/packages/component-base/test/dir-mixin.test.js
+++ b/packages/component-base/test/dir-mixin.test.js
@@ -221,6 +221,15 @@ const runTests = (baseClass) => {
       await Promise.resolve();
       expect(element.getAttribute('dir')).to.eql('ltr');
     });
+
+    it('should keep custom direction when disconnecting and reconnecting', async () => {
+      element.setAttribute('dir', 'ltr');
+      setDir('rtl');
+      await Promise.resolve();
+      // Disconnect + reconnect
+      document.body.appendChild(element);
+      expect(element.getAttribute('dir')).to.eql('ltr');
+    });
   });
 };