Skip to content

Commit 78f09bf

Browse files
ja-nimarlonicus
andcommitted
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 <[email protected]>
1 parent 84b410a commit 78f09bf

File tree

4 files changed

+35
-4
lines changed

4 files changed

+35
-4
lines changed

.eslintrc.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ module.exports = {
2828
'import/no-cycle': 'error',
2929
'no-unreachable': 'error',
3030
'no-undef': 'error',
31-
'eqeqeq': 'error',
31+
eqeqeq: 'error',
3232
'react-hooks/rules-of-hooks': 'error',
3333
'react-hooks/exhaustive-deps': 'error',
3434
'react/jsx-uses-react': 'error',

.github/workflows/benchmarking.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ jobs:
3636
node-version-file: '.nvmrc'
3737
cache: 'yarn'
3838
- run: yarn install --immutable
39-
- run: cd examples/benchmarking && yarn build && yarn compare mountFacetDomFiber mountReactDom 90
39+
- run: cd examples/benchmarking && yarn build && yarn compare mountFacetDomFiber mountReactDom 91
4040

4141
overhead:
4242
runs-on: ubuntu-latest

CONTRIBUTING.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
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.
44

5-
- 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.
5+
- 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.
66
- Make sure that you are logged in into your `npm` account. Use the command `yarn npm login` on the project folder to do this.
77
- While on the `main` branch.
88
- 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.
2525
- 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.
2626
- Push commit and the tag `git push --follow-tags`.
2727
- Publish the packages by running `yarn publish --tag rc` (it will also build the packages).
28-
- We are currently not doing release notes for release candidates so you are all done! 🎉
28+
- We are currently not doing release notes for release candidates so you are all done! 🎉

packages/@react-facet/dom-fiber/src/setupHostConfig.spec.tsx

+31
Original file line numberDiff line numberDiff line change
@@ -2199,6 +2199,37 @@ describe('umnount', () => {
21992199
expect(unsubscribe).toHaveBeenCalledTimes(1)
22002200
})
22012201

2202+
it('unsubscribes from a facet (via a fast-* component) inserted using insertBefore, when the parent is unmounted', () => {
2203+
const unsubscribe = jest.fn()
2204+
2205+
const facet: Facet<string> = {
2206+
get: () => 'abc',
2207+
observe: jest.fn().mockReturnValue(unsubscribe),
2208+
}
2209+
2210+
const TestComponent = ({ show, facet }: { facet: Facet<string>; show?: boolean }) => (
2211+
<div>
2212+
{show ? <fast-text text={facet} /> : null}
2213+
<div />
2214+
</div>
2215+
)
2216+
2217+
render(<TestComponent facet={facet} />)
2218+
2219+
expect(facet.observe).toHaveBeenCalledTimes(0)
2220+
expect(unsubscribe).toHaveBeenCalledTimes(0)
2221+
2222+
render(<TestComponent facet={facet} show />)
2223+
2224+
expect(facet.observe).toHaveBeenCalledTimes(1)
2225+
expect(unsubscribe).toHaveBeenCalledTimes(0)
2226+
2227+
render(<></>)
2228+
2229+
expect(facet.observe).toHaveBeenCalledTimes(1)
2230+
expect(unsubscribe).toHaveBeenCalledTimes(1)
2231+
})
2232+
22022233
it('keeps the subscription of facets when moving in a keyed list', () => {
22032234
const unsubscribeA = jest.fn()
22042235

0 commit comments

Comments
 (0)