changed ReactChildrenMutationWarningHook to Object.freeze (#7455)
- only freeze children array created by createElement
Keyan Zhang committed
Sep 13, 2016 at 09:25 UTC
38c4ade6cc2588e28090385ec989997363bcb782
5 files changed
+50
-121
src/isomorphic/classic/element/ReactElement.js
+5
-8
@@ -135,7 +135,6 @@ var ReactElement = function(type, key, ref, self, source, owner, props) {
135
// This can be replaced with a WeakMap once they are implemented in
136
// commonly used development environments.
137
element._store = {};
138
- var shadowChildren = Array.isArray(props.children) ? props.children.slice(0) : props.children;
138
139
// To make comparing ReactElements easier for testing purposes, we make
140
// the validation flag non-enumerable (where possible, which should
@@ -155,12 +154,6 @@ var ReactElement = function(type, key, ref, self, source, owner, props) {
154
writable: false,
155
value: self,
156
});
158
- Object.defineProperty(element, '_shadowChildren', {
159
- configurable: false,
160
- enumerable: false,
161
- writable: false,
162
- value: shadowChildren,
163
- });
157
// Two elements created in two different places should be considered
158
// equal for testing purposes and therefore we hide it from enumeration.
159
Object.defineProperty(element, '_source', {
@@ -172,7 +165,6 @@ var ReactElement = function(type, key, ref, self, source, owner, props) {
165
} else {
166
element._store.validated = false;
167
element._self = self;
175
- element._shadowChildren = shadowChildren;
168
element._source = source;
169
}
170
if (Object.freeze) {
@@ -238,6 +230,11 @@ ReactElement.createElement = function(type, config, children) {
230
for (var i = 0; i < childrenLength; i++) {
231
childArray[i] = arguments[i + 2];
232
}
233
+ if (__DEV__) {
234
+ if (Object.freeze) {
235
+ Object.freeze(childArray);
236
+ }
237
+ }
238
props.children = childArray;
239
}
240
src/renderers/dom/server/__tests__/ReactServerRendering-test.js
+10
-27
@@ -539,37 +539,20 @@ describe('ReactServerRendering', () => {
539
expect(markup).toBe('<div></div>');
540
});
541
542
- it('warns when children are mutated before render', () => {
543
- function normalizeCodeLocInfo(str) {
544
- return str.replace(/\(at .+?:\d+\)/g, '(at **)');
545
- }
546
-
542
+ it('should warn when children are mutated during render', () => {
543
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', () => {
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} />];
544
function Wrapper(props) {
545
props.children[1] = <p key={1} />; // Mutation is illegal
546
return <div>{props.children}</div>;
547
}
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
- );
548
+ expect(() => {
549
+ ReactServerRendering.renderToStaticMarkup(
550
+ <Wrapper>
551
+ <span key={0}/>
552
+ <span key={1}/>
553
+ <span key={2}/>
554
+ </Wrapper>
555
+ );
556
+ }).toThrowError(/Cannot assign to read only property.*/);
557
});
558
});
src/renderers/shared/ReactDebugTool.js
-2
@@ -15,7 +15,6 @@
15
var ReactInvalidSetStateWarningHook = require('ReactInvalidSetStateWarningHook');
16
var ReactHostOperationHistoryHook = require('ReactHostOperationHistoryHook');
17
var ReactComponentTreeHook = require('ReactComponentTreeHook');
18
-var ReactChildrenMutationWarningHook = require('ReactChildrenMutationWarningHook');
18
var ExecutionEnvironment = require('ExecutionEnvironment');
19
20
var performanceNow = require('performanceNow');
@@ -417,7 +416,6 @@ var ReactDebugTool = {
416
417
ReactDebugTool.addHook(ReactInvalidSetStateWarningHook);
418
ReactDebugTool.addHook(ReactComponentTreeHook);
420
-ReactDebugTool.addHook(ReactChildrenMutationWarningHook);
419
var url = (ExecutionEnvironment.canUseDOM && window.location.href) || '';
420
if ((/[?&]react_perf\b/).test(url)) {
421
ReactDebugTool.beginProfiling();
src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js
deleted
-61
@@ -1,61 +0,0 @@
1
-/**
2
- * Copyright 2013-present, Facebook, Inc.
3
- * All rights reserved.
4
- *
5
- * This source code is licensed under the BSD-style license found in the
6
- * LICENSE file in the root directory of this source tree. An additional grant
7
- * of patent rights can be found in the PATENTS file in the same directory.
8
- *
9
- * @providesModule ReactChildrenMutationWarningHook
10
- * @flow
11
- */
12
-
13
-'use strict';
14
-
15
-var ReactComponentTreeHook = require('ReactComponentTreeHook');
16
-
17
-var warning = require('warning');
18
-
19
-import type { DebugID } from 'ReactInstanceType';
20
-
21
-function handleElement(debugID, element) {
22
- if (element == null) {
23
- return;
24
- }
25
- if (element._shadowChildren === undefined) {
26
- return;
27
- }
28
- if (element._shadowChildren === element.props.children) {
29
- return;
30
- }
31
- var isMutated = false;
32
- if (Array.isArray(element._shadowChildren)) {
33
- if (element._shadowChildren.length === element.props.children.length) {
34
- for (var i = 0; i < element._shadowChildren.length; i++) {
35
- if (element._shadowChildren[i] !== element.props.children[i]) {
36
- isMutated = true;
37
- }
38
- }
39
- } else {
40
- isMutated = true;
41
- }
42
- }
43
- if (!Array.isArray(element._shadowChildren) || isMutated) {
44
- warning(
45
- false,
46
- 'Component\'s children should not be mutated.%s',
47
- ReactComponentTreeHook.getStackAddendumByID(debugID),
48
- );
49
- }
50
-}
51
-
52
-var ReactChildrenMutationWarningHook = {
53
- onMountComponent(debugID: DebugID): void {
54
- handleElement(debugID, ReactComponentTreeHook.getElement(debugID));
55
- },
56
- onUpdateComponent(debugID: DebugID): void {
57
- handleElement(debugID, ReactComponentTreeHook.getElement(debugID));
58
- },
59
-};
60
-
61
-module.exports = ReactChildrenMutationWarningHook;
src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js
+35
-23
@@ -15,10 +15,6 @@ var React;
15
var ReactDOM;
16
var ReactTestUtils;
17
18
-function normalizeCodeLocInfo(str) {
19
- return str.replace(/\(at .+?:\d+\)/g, '(at **)');
20
-}
21
-
18
describe('ReactComponent', () => {
19
beforeEach(() => {
20
React = require('React');
@@ -49,30 +45,46 @@ describe('ReactComponent', () => {
45
}).toThrow();
46
});
47
52
- it('should warn when children are mutated before render', () => {
53
- spyOn(console, 'error');
54
- var children = [<span key={0} />, <span key={1} />, <span key={2} />];
55
- var element = <div>{children}</div>;
56
- children[1] = <p key={1} />; // Mutation is illegal
57
- ReactTestUtils.renderIntoDocument(element);
58
- expect(console.error.calls.count()).toBe(1);
59
- expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
60
- 'Warning: Component\'s children should not be mutated.\n in div (at **)'
61
- );
62
- });
63
-
64
- it('should warn when children are mutated', () => {
48
+ it('should warn when children are mutated during render', () => {
49
spyOn(console, 'error');
66
- var children = [<span key={0} />, <span key={1} />, <span key={2} />];
50
function Wrapper(props) {
51
props.children[1] = <p key={1} />; // Mutation is illegal
52
return <div>{props.children}</div>;
53
}
71
- ReactTestUtils.renderIntoDocument(<Wrapper>{children}</Wrapper>);
72
- expect(console.error.calls.count()).toBe(1);
73
- expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
74
- 'Warning: Component\'s children should not be mutated.\n in Wrapper (at **)'
75
- );
54
+ expect(() => {
55
+ ReactTestUtils.renderIntoDocument(
56
+ <Wrapper>
57
+ <span key={0}/>
58
+ <span key={1}/>
59
+ <span key={2}/>
60
+ </Wrapper>
61
+ );
62
+ }).toThrowError(/Cannot assign to read only property.*/);
63
+ });
64
+
65
+ it('should warn when children are mutated during update', () => {
66
+ spyOn(console, 'error');
67
+
68
+ class Wrapper extends React.Component {
69
+ componentDidMount() {
70
+ this.props.children[1] = <p key={1} />; // Mutation is illegal
71
+ this.forceUpdate();
72
+ }
73
+
74
+ render() {
75
+ return <div>{this.props.children}</div>;
76
+ }
77
+ }
78
+
79
+ expect(() => {
80
+ ReactTestUtils.renderIntoDocument(
81
+ <Wrapper>
82
+ <span key={0}/>
83
+ <span key={1}/>
84
+ <span key={2}/>
85
+ </Wrapper>
86
+ );
87
+ }).toThrowError(/Cannot assign to read only property.*/);
88
});
89
90
it('should support refs on owned components', () => {