From 9b98b4fdb6960a38ef7754963791aa00044f56ce Mon Sep 17 00:00:00 2001
From: James N <104491929+ja-ni@users.noreply.github.com>
Date: Thu, 16 May 2024 10:22:11 +0200
Subject: [PATCH] Fix memory leak within the @react-facet/dom-fiber (#136)

* reverts package json changes

* reverts package json changes again...

* Add unit test for unsubscribing from insertBefore/fast-* component.

* updates performance benchmark, formats code

* removes only test and deferred mount override

* updates broken test file

---------

Co-authored-by: Marlon Huber-Smith <marlonhubersmith@gmail.com>
---
 .eslintrc.js                                  |  2 +-
 .github/workflows/benchmarking.yml            |  2 +-
 CONTRIBUTING.md                               |  4 +--
 .../dom-fiber/src/setupHostConfig.spec.tsx    | 31 +++++++++++++++++++
 tsconfig.json                                 | 10 +++---
 5 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/.eslintrc.js b/.eslintrc.js
index 2bcd49e7..9e9704da 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -28,7 +28,7 @@ module.exports = {
     'import/no-cycle': 'error',
     'no-unreachable': 'error',
     'no-undef': 'error',
-    'eqeqeq': 'error',
+    eqeqeq: 'error',
     'react-hooks/rules-of-hooks': 'error',
     'react-hooks/exhaustive-deps': 'error',
     'react/jsx-uses-react': 'error',
diff --git a/.github/workflows/benchmarking.yml b/.github/workflows/benchmarking.yml
index 3ffab68c..0cab045a 100644
--- a/.github/workflows/benchmarking.yml
+++ b/.github/workflows/benchmarking.yml
@@ -36,7 +36,7 @@ jobs:
           node-version-file: '.nvmrc'
           cache: 'yarn'
       - run: yarn install --immutable
-      - run: cd examples/benchmarking && yarn build && yarn compare mountFacetDomFiber mountReactDom 90
+      - run: cd examples/benchmarking && yarn build && yarn compare mountFacetDomFiber mountReactDom 91
 
   overhead:
     runs-on: ubuntu-latest
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 0a70bf6e..191cae8c 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -2,7 +2,7 @@
 
 Currently the process of updating the version of the packages is mostly manual. Before you start, first you need to make sure you have the permissions to publish the packages.
 
-- If there is a release branch (see __Release candidate__ below) that hasn't been merged to `main` yet now is the time to do so.
+- If there is a release branch (see **Release candidate** below) that hasn't been merged to `main` yet now is the time to do so.
 - Make sure that you are logged in into your `npm` account. Use the command `yarn npm login` on the project folder to do this.
 - While on the `main` branch.
 - Perform a search an replace on all "package.json" files from the old version to the new. (ex `0.1.4` to `0.2.0`).
@@ -25,4 +25,4 @@ Currently the process of updating the version of the packages is mostly manual.
 - Create an annotated git tag by running `git tag -a v0.4.0-rc.0` (replace with the version). The tag message can be the version again.
 - Push commit and the tag `git push --follow-tags`.
 - Publish the packages by running `yarn publish --tag rc` (it will also build the packages).
-- We are currently not doing release notes for release candidates so you are all done! 🎉
\ No newline at end of file
+- We are currently not doing release notes for release candidates so you are all done! 🎉
diff --git a/packages/@react-facet/dom-fiber/src/setupHostConfig.spec.tsx b/packages/@react-facet/dom-fiber/src/setupHostConfig.spec.tsx
index 3ae5a67a..78b32a94 100644
--- a/packages/@react-facet/dom-fiber/src/setupHostConfig.spec.tsx
+++ b/packages/@react-facet/dom-fiber/src/setupHostConfig.spec.tsx
@@ -2041,6 +2041,37 @@ describe('umnount', () => {
     expect(unsubscribe).toHaveBeenCalledTimes(1)
   })
 
+  it('unsubscribes from a facet (via a fast-* component) inserted using insertBefore, when the parent is unmounted', () => {
+    const unsubscribe = jest.fn()
+
+    const facet: Facet<string> = {
+      get: () => 'abc',
+      observe: jest.fn().mockReturnValue(unsubscribe),
+    }
+
+    const TestComponent = ({ show, facet }: { facet: Facet<string>; show?: boolean }) => (
+      <div>
+        {show ? <fast-text text={facet} /> : null}
+        <div />
+      </div>
+    )
+
+    render(<TestComponent facet={facet} />)
+
+    expect(facet.observe).toHaveBeenCalledTimes(0)
+    expect(unsubscribe).toHaveBeenCalledTimes(0)
+
+    render(<TestComponent facet={facet} show />)
+
+    expect(facet.observe).toHaveBeenCalledTimes(1)
+    expect(unsubscribe).toHaveBeenCalledTimes(0)
+
+    render(<></>)
+
+    expect(facet.observe).toHaveBeenCalledTimes(1)
+    expect(unsubscribe).toHaveBeenCalledTimes(1)
+  })
+
   it('keeps the subscription of facets when moving in a keyed list', () => {
     const unsubscribeA = jest.fn()
 
diff --git a/tsconfig.json b/tsconfig.json
index a711293f..d4840c40 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -1,11 +1,11 @@
 {
   "compilerOptions": {
     "baseUrl": ".",
-		"paths": {
-			"@react-facet/core": ["packages/@react-facet/core/*"],
-			"@react-facet/dom-fiber": ["packages/@react-facet/dom-fiber/*"],
-			"@react-facet/dom-fiber-testing-library": ["packages/@react-facet/dom-fiber-testing-library/*"]
-		},
+    "paths": {
+      "@react-facet/core": ["packages/@react-facet/core/*"],
+      "@react-facet/dom-fiber": ["packages/@react-facet/dom-fiber/*"],
+      "@react-facet/dom-fiber-testing-library": ["packages/@react-facet/dom-fiber-testing-library/*"]
+    },
     "outDir": "dist",
     "target": "es6",
     "moduleResolution": "node",