From bb37b80f0095ca1ac276bc384f1b9e933d2e13db Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 8 Nov 2018 14:08:04 +0000 Subject: [PATCH] A possible fix to the memory leak issues with useEffect Revert changes to unmount of fiber Fix Flow errors Fix prettier Add a __key field to create functions to check identity revert change --- .../src/ReactFiberCommitWork.js | 3 ++- .../react-reconciler/src/ReactFiberHooks.js | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index f22e3efb5d717..51269661ffed0 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -316,7 +316,8 @@ function commitHookEffectList( } if ((effect.tag & mountTag) !== NoHookEffect) { // Mount - const create = effect.create; + const create = ((effect.create: any): () => mixed); + effect.create = null; let destroy = create(); if (typeof destroy !== 'function') { if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index c63ead19d983b..115a8a355cedc 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -62,7 +62,7 @@ export type Hook = { type Effect = { tag: HookEffectTag, - create: () => mixed, + create: null | (() => mixed), destroy: (() => mixed) | null, inputs: Array, next: Effect, @@ -552,7 +552,20 @@ function useEffectImpl(fiberEffectTag, hookEffectTag, create, inputs): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); workInProgressHook = createWorkInProgressHook(); - let nextInputs = inputs !== undefined && inputs !== null ? inputs : [create]; + let nextInputs; + if (inputs !== undefined && inputs !== null) { + nextInputs = inputs; + } else { + // Rather than storing the create function in the nextInputs, which might + // cause memory issues due to shared context of the retained closure. We + // can instead add field called "__key" and assign a symbol to the field. + // We then store this symbol and use it as a key instead of the function. + // Note: This will require the ES2015 Symbol polyfill. + if (create.__key === undefined) { + create.__key = Symbol(); + } + nextInputs = [create.__key]; + } let destroy = null; if (currentHook !== null) { const prevEffect = currentHook.memoizedState;