Skip to content

Commit

Permalink
A possible fix to the memory leak issues with useEffect
Browse files Browse the repository at this point in the history
Revert changes to unmount of fiber

Fix Flow errors

Fix prettier

Add a __key field to create functions to check identity

revert change
  • Loading branch information
trueadm committed Nov 8, 2018
1 parent 051272f commit bb37b80
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
3 changes: 2 additions & 1 deletion packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__) {
Expand Down
17 changes: 15 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export type Hook = {

type Effect = {
tag: HookEffectTag,
create: () => mixed,
create: null | (() => mixed),
destroy: (() => mixed) | null,
inputs: Array<mixed>,
next: Effect,
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit bb37b80

Please sign in to comment.