Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

Use Finalization registry to dispose reactions of uncommitted components #332

Merged
merged 12 commits into from
Nov 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/nasty-rats-applaud.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"mobx-react-lite": patch
---

Introduce alternative way for managing cleanup of reactions.
This is internal change and shouldn't affect functionality of the library.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version: 2.1
executors:
my-executor:
docker:
- image: circleci/node:11
- image: circleci/node:14
danielkcz marked this conversation as resolved.
Show resolved Hide resolved
environment:
CI: "yes"

Expand Down
7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,18 @@ observerBatching(customBatchedUpdates)

## Testing

Running the full test suite now requires node 14+
But the library itself does not have this limitation

In order to avoid memory leaks due to aborted renders from React
fiber handling or React `StrictMode`, this library needs to
fiber handling or React `StrictMode`, on environments that does not support [FinalizationRegistry](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry), this library needs to
run timers to tidy up the remains of the aborted renders.

This can cause issues with test frameworks such as Jest
which require that timers be cleaned up before the tests
can exit.

### **`clearTimers()**
### **`clearTimers()`**

Call `clearTimers()` in the `afterEach` of your tests to ensure
that `mobx-react-lite` cleans up immediately and allows tests
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,15 @@
"@testing-library/react": "9.3.0",
"@testing-library/react-hooks": "1.1.0",
"@types/jest": "24.0.19",
"@types/node": "12.7.12",
"@types/node": "14.14.6",
"@types/react": "16.9.6",
"@types/react-dom": "16.9.2",
"@typescript-eslint/eslint-plugin": "^2.19.2",
"@typescript-eslint/parser": "^2.19.2",
"coveralls": "3.0.7",
"eslint": "^6.1.0",
"eslint-plugin-react": "^7.18.3",
"expose-gc": "^1.0.0",
"husky": "3.0.9",
"jest": "24.9.0",
"jest-environment-jsdom": "24.9.0",
Expand Down
20 changes: 14 additions & 6 deletions src/useObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ import React from "react"

import { printDebugValue } from "./utils/printDebugValue"
import {
createTrackingData,
addReactionToTrack,
IReactionTracking,
recordReactionAsCommitted,
scheduleCleanupOfReactionIfLeaked
recordReactionAsCommitted
} from "./utils/reactionCleanupTracking"
import { isUsingStaticRendering } from "./staticRendering"
import { useForceUpdate } from "./utils/utils"
Expand All @@ -15,11 +14,18 @@ function observerComponentNameFor(baseComponentName: string) {
return `observer${baseComponentName}`
}

/**
* We use class to make it easier to detect in heap snapshots by name
*/
class ObjectToBeRetainedByReact {}

export function useObserver<T>(fn: () => T, baseComponentName: string = "observed"): T {
if (isUsingStaticRendering()) {
return fn()
}

const [objectRetainedByReact] = React.useState(new ObjectToBeRetainedByReact())

const forceUpdate = useForceUpdate()

// StrictMode/ConcurrentMode/Suspense may mean that our component is
Expand Down Expand Up @@ -47,9 +53,11 @@ export function useObserver<T>(fn: () => T, baseComponentName: string = "observe
}
})

const trackingData = createTrackingData(newReaction)
reactionTrackingRef.current = trackingData
scheduleCleanupOfReactionIfLeaked(reactionTrackingRef)
Bnaya marked this conversation as resolved.
Show resolved Hide resolved
const trackingData = addReactionToTrack(
reactionTrackingRef,
newReaction,
objectRetainedByReact
)
}

const { reaction } = reactionTrackingRef.current!
Expand Down
12 changes: 12 additions & 0 deletions src/utils/FinalizationRegistryWrapper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
declare class FinalizationRegistryType<T> {
constructor(cleanup: (cleanupToken: T) => void)
register(object: object, cleanupToken: T, unregisterToken?: object): void
unregister(unregisterToken: object): void
}

declare const FinalizationRegistry: typeof FinalizationRegistryType | undefined

const FinalizationRegistryLocal =
typeof FinalizationRegistry === "undefined" ? undefined : FinalizationRegistry

export { FinalizationRegistryLocal as FinalizationRegistry }
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { FinalizationRegistry as FinalizationRegistryMaybeUndefined } from "./FinalizationRegistryWrapper"
import { Reaction } from "mobx"
import {
ReactionCleanupTracking,
IReactionTracking,
createTrackingData
} from "./reactionCleanupTrackingCommon"

/**
* FinalizationRegistry-based uncommitted reaction cleanup
*/
export function createReactionCleanupTrackingUsingFinalizationRegister(
FinalizationRegistry: NonNullable<typeof FinalizationRegistryMaybeUndefined>
): ReactionCleanupTracking {
const cleanupTokenToReactionTrackingMap = new Map<number, IReactionTracking>()
let globalCleanupTokensCounter = 1

const registry = new FinalizationRegistry(function cleanupFunction(token: number) {
const trackedReaction = cleanupTokenToReactionTrackingMap.get(token)
if (trackedReaction) {
trackedReaction.reaction.dispose()
cleanupTokenToReactionTrackingMap.delete(token)
}
})

return {
addReactionToTrack(
reactionTrackingRef: React.MutableRefObject<IReactionTracking | null>,
reaction: Reaction,
objectRetainedByReact: object
) {
const token = globalCleanupTokensCounter++

registry.register(objectRetainedByReact, token, reactionTrackingRef)
reactionTrackingRef.current = createTrackingData(reaction)
reactionTrackingRef.current.finalizationRegistryCleanupToken = token
cleanupTokenToReactionTrackingMap.set(token, reactionTrackingRef.current)

return reactionTrackingRef.current
},
recordReactionAsCommitted(reactionRef: React.MutableRefObject<IReactionTracking | null>) {
registry.unregister(reactionRef)

if (reactionRef.current && reactionRef.current.finalizationRegistryCleanupToken) {
cleanupTokenToReactionTrackingMap.delete(
reactionRef.current.finalizationRegistryCleanupToken
)
}
},
forceCleanupTimerToRunNowForTests() {
// When FinalizationRegistry in use, this this is no-op
},
resetCleanupScheduleForTests() {
// When FinalizationRegistry in use, this this is no-op
}
}
}
122 changes: 122 additions & 0 deletions src/utils/createTimerBasedReactionCleanupTracking.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import { Reaction } from "mobx"
import {
ReactionCleanupTracking,
IReactionTracking,
CLEANUP_TIMER_LOOP_MILLIS,
createTrackingData
} from "./reactionCleanupTrackingCommon"

/**
* timers, gc-style, uncommitted reaction cleanup
*/
export function createTimerBasedReactionCleanupTracking(): ReactionCleanupTracking {
/**
* Reactions created by components that have yet to be fully mounted.
*/
const uncommittedReactionRefs: Set<React.MutableRefObject<IReactionTracking | null>> = new Set()

/**
* Latest 'uncommitted reactions' cleanup timer handle.
*/
let reactionCleanupHandle: ReturnType<typeof setTimeout> | undefined

/* istanbul ignore next */
/**
* Only to be used by test functions; do not export outside of mobx-react-lite
*/
function forceCleanupTimerToRunNowForTests() {
// This allows us to control the execution of the cleanup timer
// to force it to run at awkward times in unit tests.
if (reactionCleanupHandle) {
clearTimeout(reactionCleanupHandle)
cleanUncommittedReactions()
}
}

/* istanbul ignore next */
function resetCleanupScheduleForTests() {
if (uncommittedReactionRefs.size > 0) {
for (const ref of uncommittedReactionRefs) {
const tracking = ref.current
if (tracking) {
tracking.reaction.dispose()
ref.current = null
}
}
uncommittedReactionRefs.clear()
}

if (reactionCleanupHandle) {
clearTimeout(reactionCleanupHandle)
reactionCleanupHandle = undefined
}
}

function ensureCleanupTimerRunning() {
if (reactionCleanupHandle === undefined) {
reactionCleanupHandle = setTimeout(cleanUncommittedReactions, CLEANUP_TIMER_LOOP_MILLIS)
}
}

function scheduleCleanupOfReactionIfLeaked(
ref: React.MutableRefObject<IReactionTracking | null>
) {
uncommittedReactionRefs.add(ref)

ensureCleanupTimerRunning()
}

function recordReactionAsCommitted(
reactionRef: React.MutableRefObject<IReactionTracking | null>
) {
uncommittedReactionRefs.delete(reactionRef)
}

/**
* Run by the cleanup timer to dispose any outstanding reactions
*/
function cleanUncommittedReactions() {
reactionCleanupHandle = undefined

// Loop through all the candidate leaked reactions; those older
// than CLEANUP_LEAKED_REACTIONS_AFTER_MILLIS get tidied.

const now = Date.now()
uncommittedReactionRefs.forEach(ref => {
const tracking = ref.current
if (tracking) {
if (now >= tracking.cleanAt) {
// It's time to tidy up this leaked reaction.
tracking.reaction.dispose()
ref.current = null
uncommittedReactionRefs.delete(ref)
}
}
})

if (uncommittedReactionRefs.size > 0) {
// We've just finished a round of cleanups but there are still
// some leak candidates outstanding.
ensureCleanupTimerRunning()
}
}

return {
addReactionToTrack(
reactionTrackingRef: React.MutableRefObject<IReactionTracking | null>,
reaction: Reaction,
/**
* On timer based implementation we don't really need this object,
* but we keep the same api
*/
objectRetainedByReact: unknown
) {
reactionTrackingRef.current = createTrackingData(reaction)
scheduleCleanupOfReactionIfLeaked(reactionTrackingRef)
return reactionTrackingRef.current
},
recordReactionAsCommitted,
forceCleanupTimerToRunNowForTests,
resetCleanupScheduleForTests
}
}
Loading