@samitouri / QOS-React / commits / 5514ea369d

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

Keyan Zhang committed Aug 11, 2016 at 02:50 UTC 5514ea369d7f60127ae15774e1276b21cd7c0107
2 files changed +39 -15
src/renderers/dom/server/__tests__/ReactServerRendering-test.js
+34
@@ -538,4 +538,38 @@ describe('ReactServerRendering', function() {
538 var markup = ReactServerRendering.renderToStaticMarkup(<Baz />);
539 expect(markup).toBe('<div></div>');
540 });
541 +
542 + it('warns when children are mutated before render', function() {
543 + function normalizeCodeLocInfo(str) {
544 + return str.replace(/\(at .+?:\d+\)/g, '(at **)');
545 + }
546 +
547 + spyOn(console, 'error');
548 + var children = [<span key={0} />, <span key={1} />, <span key={2} />];
549 + var element = <div>{children}</div>;
550 + children[1] = <p key={1} />; // Mutation is illegal
551 + ReactServerRendering.renderToString(element);
552 + expect(console.error.calls.count()).toBe(1);
553 + expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
554 + 'Warning: Component\'s children should not be mutated.\n in div (at **)'
555 + );
556 + });
557 +
558 + it('should warn when children are mutated', function() {
559 + function normalizeCodeLocInfo(str) {
560 + return str.replace(/\(at .+?:\d+\)/g, '(at **)');
561 + }
562 +
563 + spyOn(console, 'error');
564 + var children = [<span key={0} />, <span key={1} />, <span key={2} />];
565 + function Wrapper(props) {
566 + props.children[1] = <p key={1} />; // Mutation is illegal
567 + return <div>{props.children}</div>;
568 + }
569 + ReactServerRendering.renderToString(<Wrapper>{children}</Wrapper>);
570 + expect(console.error.calls.count()).toBe(1);
571 + expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
572 + 'Warning: Component\'s children should not be mutated.\n in Wrapper (at **)'
573 + );
574 + });
575 });
src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js
+5 -15
@@ -15,8 +15,6 @@ var ReactComponentTreeHook = require('ReactComponentTreeHook');
15
16 var warning = require('warning');
17
18 -var elements = {};
19 -
18 function handleElement(debugID, element) {
19 if (element == null) {
20 return;
@@ -46,21 +44,13 @@ function handleElement(debugID, element) {
44 );
45 }
46
49 -var ReactDOMUnknownPropertyHook = {
50 - onBeforeMountComponent(debugID, element) {
51 - elements[debugID] = element;
52 - },
53 - onBeforeUpdateComponent(debugID, element) {
54 - elements[debugID] = element;
55 - },
56 - onComponentHasMounted(debugID) {
57 - handleElement(debugID, elements[debugID]);
58 - delete elements[debugID];
47 +var ReactChildrenMutationWarningHook = {
48 + onMountComponent(debugID) {
49 + handleElement(debugID, ReactComponentTreeHook.getElement(debugID));
50 },
51 onComponentHasUpdated(debugID) {
61 - handleElement(debugID, elements[debugID]);
62 - delete elements[debugID];
52 + handleElement(debugID, ReactComponentTreeHook.getElement(debugID));
53 },
54 };
55
66 -module.exports = ReactDOMUnknownPropertyHook;
56 +module.exports = ReactChildrenMutationWarningHook;