[#720] Add strict mode compatibility

Fixes #720 In React strict mode, the trap gets immediately unmounted and remounted after first mount. The trap deactivates automatically on unmount, so on remount, we try to restore the trap to its previous active/paused state based on the component's existing state. React does not re-render the component, nor does it call callback refs again, so we have no choice but to assume the DOM hasn't changed and the existing trap's container elements are still in the DOM... Way to go React for strict mode violating the assumption of `componentWillUnmount()`, "Once a component is unmounted, it will never be mounted again." Not a fan. Also bumped react-dom to 18.2.0 since react was already there, and set NODE_ENV for different builds.
focus-trap · Jun 18, 2022 · 6d2c92c · 6d2c92c
1 parent 8beb07e
commit 6d2c92c
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 54 deletions.
diff --git a/.changeset/happy-knives-glow.md b/.changeset/happy-knives-glow.md
@@ -0,0 +1,5 @@
+---
+'focus-trap-react': patch
+---
+
+Fix an issue when running in strict mode which has React immediately unmount/remount the trap, causing it to deactivate and then have to reactivate (per existing component state) on the remount. [#720](https://github.com/focus-trap/focus-trap-react/issues/720)
diff --git a/demo/index.html b/demo/index.html
@@ -70,7 +70,7 @@ <h2 id="special-element-heading">special element</h2>
       arbitrary <code>data-</code>attribute.
     </p>
     <p>
-      Also, this FocusTrap activates and deactivates while staying mounted.
+      Also, this FocusTrap activates and deactivates while staying mounted (and does so in <a href="https://reactjs.org/docs/strict-mode.html" target="_blank">React 18 Strict Mode</a>, though the true effects of this are only applicable if you're running this demo <strong>locally in development mode</strong> since strict mode does not affect production code).
     </p>
     <p>
       ALSO, when you click outside this FocusTrap, the trap deactivates and your click carries through.

diff --git a/demo/js/demo-special-element.js b/demo/js/demo-special-element.js
@@ -9,7 +9,7 @@ class DemoSpecialElement extends React.Component {
     super(props);
 
     this.state = {
-      activeTrap: false,
+      activeTrap: true,
       passThruMsg: '',
     };
 
@@ -37,46 +37,48 @@ class DemoSpecialElement extends React.Component {
     }
 
     return (
-      <div>
-        <p>
-          <button
-            onClick={this.mountTrap}
-            aria-describedby="special-element-heading"
+      <React.StrictMode>
+        <div>
+          <p>
+            <button
+              onClick={this.mountTrap}
+              aria-describedby="special-element-heading"
+            >
+              activate trap
+            </button>
+            <button onClick={this.updatePassThruMsg}>pass thru click</button>
+            <span>{this.state.passThruMsg}</span>
+          </p>
+          <FocusTrap
+            active={this.state.activeTrap}
+            focusTrapOptions={{
+              onPostDeactivate: this.unmountTrap,
+              clickOutsideDeactivates: true,
+              returnFocusOnDeactivate: true,
+            }}
           >
-            activate trap
-          </button>
-          <button onClick={this.updatePassThruMsg}>pass thru click</button>
-          <span>{this.state.passThruMsg}</span>
-        </p>
-        <FocusTrap
-          active={this.state.activeTrap}
-          focusTrapOptions={{
-            onPostDeactivate: this.unmountTrap,
-            clickOutsideDeactivates: true,
-            returnFocusOnDeactivate: true,
-          }}
-        >
-          <section
-            id="focus-trap-three"
-            style={this.state.activeTrap ? null : { background: '#eee' }}
-            data-whatever="nothing"
-            className={trapClass}
-          >
-            <p>
-              Here is a focus trap <a href="#">with</a> <a href="#">some</a>{' '}
-              <a href="#">focusable</a> parts.
-            </p>
-            <p>
-              <button
-                onClick={this.unmountTrap}
-                aria-describedby="special-element-heading"
-              >
-                deactivate trap
-              </button>
-            </p>
-          </section>
-        </FocusTrap>
-      </div>
+            <section
+              id="focus-trap-three"
+              style={this.state.activeTrap ? null : { background: '#eee' }}
+              data-whatever="nothing"
+              className={trapClass}
+            >
+              <p>
+                Here is a focus trap <a href="#">with</a> <a href="#">some</a>{' '}
+                <a href="#">focusable</a> parts.
+              </p>
+              <p>
+                <button
+                  onClick={this.unmountTrap}
+                  aria-describedby="special-element-heading"
+                >
+                  deactivate trap
+                </button>
+              </p>
+            </section>
+          </FocusTrap>
+        </div>
+      </React.StrictMode>
     );
   }
 }

diff --git a/package.json b/package.json
@@ -14,8 +14,8 @@
     "index.d.ts"
   ],
   "scripts": {
-    "demo-bundle": "browserify demo/js -t babelify --extension=.jsx -o demo/demo-bundle.js",
-    "start": "yarn build && budo demo/js/index.js:demo-bundle.js --dir demo --live -- -t babelify --extension=.jsx",
+    "demo-bundle": "NODE_ENV=production browserify demo/js -t babelify --extension=.jsx -o demo/demo-bundle.js",
+    "start": "yarn build && NODE_ENV=development budo demo/js/index.js:demo-bundle.js --dir demo --live -- -t babelify --extension=.jsx",
     "lint": "eslint \"*.js\" \"src/**/*.js\" \"test/**/*.js\" \"demo/**/*.js\" \"cypress/**/*.js\"",
     "format": "prettier --write \"{*,src/**/*,test/**/*,demo/js/**/*,.github/workflows/*,cypress/**/*}.+(js|yml)\"",
     "format:check": "prettier --check \"{*,src/**/*,test/**/*,demo/js/**/*,.github/workflows/*,cypress/**/*}.+(js|yml)\"",
@@ -89,7 +89,7 @@
     "prettier": "^2.7.0",
     "prop-types": "^15.8.1",
     "react": "^18.2.0",
-    "react-dom": "^18.1.0",
+    "react-dom": "^18.2.0",
     "regenerator-runtime": "^0.13.9",
     "start-server-and-test": "^1.14.0",
     "typescript": "^4.7.3"

diff --git a/src/focus-trap-react.js b/src/focus-trap-react.js
@@ -270,7 +270,27 @@ class FocusTrap extends React.Component {
   }
 
   setupFocusTrap() {
-    if (!this.focusTrap) {
+    if (this.focusTrap) {
+      // trap already exists: it's possible we're in StrictMode and we're being remounted,
+      //  in which case, we will have deactivated the trap when we got unmounted (remember,
+      //  StrictMode, in development, purposely unmounts and remounts components after
+      //  mounting them the first time to make sure they have reusable state,
+      //  @see https://reactjs.org/docs/strict-mode.html#ensuring-reusable-state) so now
+      //  we need to restore the state of the trap according to our component state
+      // NOTE: Strict mode __violates__ assumptions about the `componentWillUnmount()` API
+      //  which clearly states -- even for React 18 -- that, "Once a component instance is
+      //  unmounted, __it will never be mounted again.__" (emphasis ours). So when we get
+      //  unmounted, we assume we're gone forever and we deactivate the trap. But then
+      //  we get remounted and we're supposed to restore state. But if you had paused,
+      //  we've now deactivated (we don't know we're amount to get remounted again)
+      //  which means we need to reactivate and then pause. Otherwise, do nothing.
+      if (this.props.active && !this.focusTrap.active) {
+        this.focusTrap.activate();
+        if (this.props.paused) {
+          this.focusTrap.pause();
+        }
+      }
+    } else {
       const nodesExist = this.focusTrapElements.some(Boolean);
       if (nodesExist) {
         // eslint-disable-next-line react/prop-types -- _createFocusTrap is an internal prop

diff --git a/yarn.lock b/yarn.lock
@@ -7504,13 +7504,13 @@ range-parser@~1.2.1:
   resolved "https://registry.yarnpkg.com/range-parser/-/range-parser-1.2.1.tgz#3cf37023d199e1c24d1a55b84800c2f3e6468031"
   integrity sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg==
 
-react-dom@^18.1.0:
-  version "18.1.0"
-  resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.1.0.tgz#7f6dd84b706408adde05e1df575b3a024d7e8a2f"
-  integrity sha512-fU1Txz7Budmvamp7bshe4Zi32d0ll7ect+ccxNu9FlObT605GOEB8BfO4tmRJ39R5Zj831VCpvQ05QPBW5yb+w==
+react-dom@^18.2.0:
+  version "18.2.0"
+  resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.2.0.tgz#22aaf38708db2674ed9ada224ca4aa708d821e3d"
+  integrity sha512-6IMTriUmvsjHUjNtEDudZfuDQUoWXVxKHhlEGSk81n4YFS+r/Kl99wXiwlVXtPBtJenozv2P+hxDsw9eA7Xo6g==
   dependencies:
     loose-envify "^1.1.0"
-    scheduler "^0.22.0"
+    scheduler "^0.23.0"
 
 react-is@^16.13.1:
   version "16.13.1"
@@ -7920,10 +7920,10 @@ saxes@^5.0.1:
   dependencies:
     xmlchars "^2.2.0"
 
-scheduler@^0.22.0:
-  version "0.22.0"
-  resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.22.0.tgz#83a5d63594edf074add9a7198b1bae76c3db01b8"
-  integrity sha512-6QAm1BgQI88NPYymgGQLCZgvep4FyePDWFpXVK+zNSUgHwlqpJy8VEh8Et0KxTACS4VWwMousBElAZOH9nkkoQ==
+scheduler@^0.23.0:
+  version "0.23.0"
+  resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.23.0.tgz#ba8041afc3d30eb206a487b6b384002e4e61fdfe"
+  integrity sha512-CtuThmgHNg7zIZWAXi3AsyIzA3n4xx7aNyjwC2VJldO2LMVDhFK+63xGqq6CsJH4rTAt6/M+N4GhZiDYPx9eUw==
   dependencies:
     loose-envify "^1.1.0"