From 38914dfe6a7e0336954c486a651e83e97a7846c0 Mon Sep 17 00:00:00 2001 From: Keyan Zhang Date: Thu, 11 Aug 2016 02:50:18 +0800 Subject: [PATCH] Fix memory leak in ReactChildrenMutationWarningHook for SSR (#7410) * corrected ReactChildrenMutationWarningHook's name * changed `onComponentHasMounted` to `onMountComponent` and get element from `ReactComponentTreeHook` instead of keeping an internal store (cherry picked from commit 5514ea369d7f60127ae15774e1276b21cd7c0107) --- .../__tests__/ReactServerRendering-test.js | 34 +++++++++++++++++++ .../hooks/ReactChildrenMutationWarningHook.js | 20 +++-------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/renderers/dom/server/__tests__/ReactServerRendering-test.js b/src/renderers/dom/server/__tests__/ReactServerRendering-test.js index e552678c8a6c4..1c3753028a3e6 100644 --- a/src/renderers/dom/server/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/server/__tests__/ReactServerRendering-test.js @@ -538,4 +538,38 @@ describe('ReactServerRendering', function() { var markup = ReactServerRendering.renderToStaticMarkup(); expect(markup).toBe('
'); }); + + it('warns when children are mutated before render', function() { + function normalizeCodeLocInfo(str) { + return str.replace(/\(at .+?:\d+\)/g, '(at **)'); + } + + spyOn(console, 'error'); + var children = [, , ]; + var element =
{children}
; + children[1] =

; // Mutation is illegal + ReactServerRendering.renderToString(element); + expect(console.error.calls.count()).toBe(1); + expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( + 'Warning: Component\'s children should not be mutated.\n in div (at **)' + ); + }); + + it('should warn when children are mutated', function() { + function normalizeCodeLocInfo(str) { + return str.replace(/\(at .+?:\d+\)/g, '(at **)'); + } + + spyOn(console, 'error'); + var children = [, , ]; + function Wrapper(props) { + props.children[1] =

; // Mutation is illegal + return

{props.children}
; + } + ReactServerRendering.renderToString({children}); + expect(console.error.calls.count()).toBe(1); + expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( + 'Warning: Component\'s children should not be mutated.\n in Wrapper (at **)' + ); + }); }); diff --git a/src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js b/src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js index fccc1538545ad..e8d063d58a9a6 100644 --- a/src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js +++ b/src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js @@ -15,8 +15,6 @@ var ReactComponentTreeHook = require('ReactComponentTreeHook'); var warning = require('warning'); -var elements = {}; - function handleElement(debugID, element) { if (element == null) { return; @@ -46,21 +44,13 @@ function handleElement(debugID, element) { ); } -var ReactDOMUnknownPropertyHook = { - onBeforeMountComponent(debugID, element) { - elements[debugID] = element; - }, - onBeforeUpdateComponent(debugID, element) { - elements[debugID] = element; - }, - onComponentHasMounted(debugID) { - handleElement(debugID, elements[debugID]); - delete elements[debugID]; +var ReactChildrenMutationWarningHook = { + onMountComponent(debugID) { + handleElement(debugID, ReactComponentTreeHook.getElement(debugID)); }, onComponentHasUpdated(debugID) { - handleElement(debugID, elements[debugID]); - delete elements[debugID]; + handleElement(debugID, ReactComponentTreeHook.getElement(debugID)); }, }; -module.exports = ReactDOMUnknownPropertyHook; +module.exports = ReactChildrenMutationWarningHook;