Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to the memory leak issue with useEffect #14153

Closed
wants to merge 2 commits into from
Closed

Fix to the memory leak issue with useEffect #14153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb37b80
A possible fix to the memory leak issues with useEffect
trueadm Nov 8, 2018
d05da56
add comment
trueadm Nov 8, 2018
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
5 changes: 4 additions & 1 deletion packages/react-reconciler/src/ReactFiberCommitWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,10 @@ function commitHookEffectList(
}
if ((effect.tag & mountTag) !== NoHookEffect) {
// Mount
const create = effect.create;
const create = ((effect.create: any): () => mixed);
// Null out the `create` field, avoiding possible memory leaks
// due to retaining the function's closure context.
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
View file
Open in desktop
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