From 3fd6484fbfe4319c464ff9b1735a1158834f9246 Mon Sep 17 00:00:00 2001 From: Keyan Zhang Date: Tue, 2 Aug 2016 14:38:02 -0700 Subject: [PATCH 1/2] corrected ReactChildrenMutationWarningHook's name --- .../shared/hooks/ReactChildrenMutationWarningHook.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js b/src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js index fccc1538545ad..ee913ccfcd260 100644 --- a/src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js +++ b/src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js @@ -46,7 +46,7 @@ function handleElement(debugID, element) { ); } -var ReactDOMUnknownPropertyHook = { +var ReactChildrenMutationWarningHook = { onBeforeMountComponent(debugID, element) { elements[debugID] = element; }, @@ -63,4 +63,4 @@ var ReactDOMUnknownPropertyHook = { }, }; -module.exports = ReactDOMUnknownPropertyHook; +module.exports = ReactChildrenMutationWarningHook; From 188a06856147056cc7ac359eca9e5f1ab604f931 Mon Sep 17 00:00:00 2001 From: Keyan Zhang Date: Tue, 2 Aug 2016 17:04:14 -0700 Subject: [PATCH 2/2] changed `onComponentHasMounted` to `onMountComponent` and get element from `ReactComponentTreeHook` instead of keeping an internal store --- .../__tests__/ReactServerRendering-test.js | 34 +++++++++++++++++++ .../hooks/ReactChildrenMutationWarningHook.js | 16 ++------- 2 files changed, 37 insertions(+), 13 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 ee913ccfcd260..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; @@ -47,19 +45,11 @@ function handleElement(debugID, element) { } var ReactChildrenMutationWarningHook = { - onBeforeMountComponent(debugID, element) { - elements[debugID] = element; - }, - onBeforeUpdateComponent(debugID, element) { - elements[debugID] = element; - }, - onComponentHasMounted(debugID) { - handleElement(debugID, elements[debugID]); - delete elements[debugID]; + onMountComponent(debugID) { + handleElement(debugID, ReactComponentTreeHook.getElement(debugID)); }, onComponentHasUpdated(debugID) { - handleElement(debugID, elements[debugID]); - delete elements[debugID]; + handleElement(debugID, ReactComponentTreeHook.getElement(debugID)); }, };