[Fiber] Complete ES6 Class related errors support (#8156)
* Print a warning in fiber for lacking render method * Reject initial non-object state in Fiber Rejects Arrays, Strings & Numbers. Allows nulls. * Centralize fiber checks on ES6 Classes * Add classic & instance property checks on fiber - check the absence of getInitialState & getDefaultProps as methods - check the absence of propTypes & contextTypes at instance-level * Convert to normalized React `warning` calls * Support lifecycle typo detection in fiber * Get the complete warnings from the existing code * Only check classInstance once Avoid rechecking while resuming * Can warn component while resuming but only once This is achieved by tracking the warning state in the Fiber structure. * Remove warning deduplication * Factor name retrieval in a getName helper * Use invariant instead of throw * Read inst.state only once for the invariant * Fix condition on the instance state * Register failing/passing fiber tests
Robin Ricard committed
Nov 5, 2016 at 19:19 UTC
ba6b53afbaa517c92cba305e1a40e42182397a44
3 files changed
+114
-16
scripts/fiber/tests-failing.txt
-16
@@ -48,30 +48,15 @@ src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js
48
* includes the owner name when passing null, undefined, boolean, or number
49
50
src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee
51
-* throws if no render function is defined
51
* renders based on context in the constructor
53
-* should throw with non-object in the initial state property
54
-* warns when classic properties are defined on the instance, but does not invoke them.
55
-* should warn when misspelling shouldComponentUpdate
56
-* should warn when misspelling componentWillReceiveProps
52
* supports this.context passed via getChildContext
53
54
src/isomorphic/modern/class/__tests__/ReactES6Class-test.js
60
-* throws if no render function is defined
55
* renders based on context in the constructor
62
-* should throw with non-object in the initial state property
63
-* warns when classic properties are defined on the instance, but does not invoke them.
64
-* should warn when misspelling shouldComponentUpdate
65
-* should warn when misspelling componentWillReceiveProps
56
* supports this.context passed via getChildContext
57
58
src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts
69
-* throws if no render function is defined
59
* renders based on context in the constructor
71
-* should throw with non-object in the initial state property
72
-* warns when classic properties are defined on the instance, but does not invoke them.
73
-* should warn when misspelling shouldComponentUpdate
74
-* should warn when misspelling componentWillReceiveProps
60
* supports this.context passed via getChildContext
61
62
src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js
@@ -538,7 +523,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js
523
* should warn about `setState` in render
524
* should warn about `setState` in getChildContext
525
* should warn when shouldComponentUpdate() returns undefined
541
-* should warn when componentDidUnmount method is defined
526
* should pass context to children when not owner
527
* should pass context when re-rendered for static child
528
* should pass context when re-rendered for static child within a composite component
scripts/fiber/tests-passing.txt
+16
@@ -350,30 +350,40 @@ src/isomorphic/modern/class/__tests__/ReactClassEquivalence-test.js
350
351
src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee
352
* preserves the name of the class for use in error messages
353
+* throws if no render function is defined
354
* renders a simple stateless component with prop
355
* renders based on state using initial values in this.props
356
* renders based on state using props in the constructor
357
* renders only once when setting state in componentWillMount
358
+* should throw with non-object in the initial state property
359
* should render with null in the initial state property
360
* setState through an event handler
361
* should not implicitly bind event handlers
362
* renders using forceUpdate even when there is no state
363
* will call all the normal life cycle methods
364
+* warns when classic properties are defined on the instance, but does not invoke them.
365
+* should warn when misspelling shouldComponentUpdate
366
+* should warn when misspelling componentWillReceiveProps
367
* should throw AND warn when trying to access classic APIs
368
* supports classic refs
369
* supports drilling through to the DOM using findDOMNode
370
371
src/isomorphic/modern/class/__tests__/ReactES6Class-test.js
372
* preserves the name of the class for use in error messages
373
+* throws if no render function is defined
374
* renders a simple stateless component with prop
375
* renders based on state using initial values in this.props
376
* renders based on state using props in the constructor
377
* renders only once when setting state in componentWillMount
378
+* should throw with non-object in the initial state property
379
* should render with null in the initial state property
380
* setState through an event handler
381
* should not implicitly bind event handlers
382
* renders using forceUpdate even when there is no state
383
* will call all the normal life cycle methods
384
+* warns when classic properties are defined on the instance, but does not invoke them.
385
+* should warn when misspelling shouldComponentUpdate
386
+* should warn when misspelling componentWillReceiveProps
387
* should throw AND warn when trying to access classic APIs
388
* supports classic refs
389
* supports drilling through to the DOM using findDOMNode
@@ -385,15 +395,20 @@ src/isomorphic/modern/class/__tests__/ReactPureComponent-test.js
395
396
src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts
397
* preserves the name of the class for use in error messages
398
+* throws if no render function is defined
399
* renders a simple stateless component with prop
400
* renders based on state using initial values in this.props
401
* renders based on state using props in the constructor
402
* renders only once when setting state in componentWillMount
403
+* should throw with non-object in the initial state property
404
* should render with null in the initial state property
405
* setState through an event handler
406
* should not implicitly bind event handlers
407
* renders using forceUpdate even when there is no state
408
* will call all the normal life cycle methods
409
+* warns when classic properties are defined on the instance, but does not invoke them.
410
+* should warn when misspelling shouldComponentUpdate
411
+* should warn when misspelling componentWillReceiveProps
412
* should throw AND warn when trying to access classic APIs
413
* supports classic refs
414
* supports drilling through to the DOM using findDOMNode
@@ -1011,6 +1026,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js
1026
* should silently allow `setState`, not call cb on unmounting components
1027
* should cleanup even if render() fatals
1028
* should call componentWillUnmount before unmounting
1029
+* should warn when componentDidUnmount method is defined
1030
* should skip update when rerendering element in container
1031
* only renders once if updated in componentWillReceiveProps
1032
* should allow access to findDOMNode in componentWillUnmount
src/renderers/shared/fiber/ReactFiberClassComponent.js
+98
@@ -24,6 +24,11 @@ var {
24
var { isMounted } = require('ReactFiberTreeReflection');
25
var ReactInstanceMap = require('ReactInstanceMap');
26
var shallowEqual = require('shallowEqual');
27
+var warning = require('warning');
28
+var invariant = require('invariant');
29
+
30
+
31
+const isArray = Array.isArray;
32
33
module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
34
@@ -91,6 +96,98 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
96
return true;
97
}
98
99
+ function getName(workInProgress: Fiber, inst: any): string {
100
+ const type = workInProgress.type;
101
+ const constructor = inst && inst.constructor;
102
+ return (
103
+ type.displayName || (constructor && constructor.displayName) ||
104
+ type.name || (constructor && constructor.name) ||
105
+ 'A Component'
106
+ );
107
+ }
108
+
109
+ function checkClassInstance(workInProgress: Fiber, inst: any) {
110
+ if (__DEV__) {
111
+ const name = getName(workInProgress, inst);
112
+ const renderPresent = inst.render;
113
+ warning(
114
+ renderPresent,
115
+ '%s(...): No `render` method found on the returned component ' +
116
+ 'instance: you may have forgotten to define `render`.',
117
+ name
118
+ );
119
+ const noGetInitialStateOnES6 = (
120
+ !inst.getInitialState ||
121
+ inst.getInitialState.isReactClassApproved
122
+ );
123
+ warning(
124
+ noGetInitialStateOnES6,
125
+ 'getInitialState was defined on %s, a plain JavaScript class. ' +
126
+ 'This is only supported for classes created using React.createClass. ' +
127
+ 'Did you mean to define a state property instead?',
128
+ name
129
+ );
130
+ const noGetDefaultPropsOnES6 = (
131
+ !inst.getDefaultProps ||
132
+ inst.getDefaultProps.isReactClassApproved
133
+ );
134
+ warning(
135
+ noGetDefaultPropsOnES6,
136
+ 'getDefaultProps was defined on %s, a plain JavaScript class. ' +
137
+ 'This is only supported for classes created using React.createClass. ' +
138
+ 'Use a static property to define defaultProps instead.',
139
+ name
140
+ );
141
+ const noInstancePropTypes = !inst.propTypes;
142
+ warning(
143
+ noInstancePropTypes,
144
+ 'propTypes was defined as an instance property on %s. Use a static ' +
145
+ 'property to define propTypes instead.',
146
+ name,
147
+ );
148
+ const noInstanceContextTypes = !inst.contextTypes;
149
+ warning(
150
+ noInstanceContextTypes,
151
+ 'contextTypes was defined as an instance property on %s. Use a static ' +
152
+ 'property to define contextTypes instead.',
153
+ name,
154
+ );
155
+ const noComponentShouldUpdate = typeof inst.componentShouldUpdate !== 'function';
156
+ warning(
157
+ noComponentShouldUpdate,
158
+ '%s has a method called ' +
159
+ 'componentShouldUpdate(). Did you mean shouldComponentUpdate()? ' +
160
+ 'The name is phrased as a question because the function is ' +
161
+ 'expected to return a value.',
162
+ name
163
+ );
164
+ const noComponentDidUnmount = typeof inst.componentDidUnmount !== 'function';
165
+ warning(
166
+ noComponentDidUnmount,
167
+ '%s has a method called ' +
168
+ 'componentDidUnmount(). But there is no such lifecycle method. ' +
169
+ 'Did you mean componentWillUnmount()?',
170
+ name
171
+ );
172
+ const noComponentWillRecieveProps = typeof inst.componentWillRecieveProps !== 'function';
173
+ warning(
174
+ noComponentWillRecieveProps,
175
+ '%s has a method called ' +
176
+ 'componentWillRecieveProps(). Did you mean componentWillReceiveProps()?',
177
+ name
178
+ );
179
+ }
180
+
181
+ const instanceState = inst.state;
182
+ if (instanceState && (typeof instanceState !== 'object' || isArray(instanceState))) {
183
+ invariant(
184
+ false,
185
+ '%s.state: must be set to an object or null',
186
+ getName(workInProgress, inst)
187
+ );
188
+ }
189
+ }
190
+
191
function adoptClassInstance(workInProgress : Fiber, instance : any) : void {
192
instance.updater = updater;
193
workInProgress.stateNode = instance;
@@ -102,6 +199,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
199
const ctor = workInProgress.type;
200
const props = workInProgress.pendingProps;
201
const instance = new ctor(props);
202
+ checkClassInstance(workInProgress, instance);
203
adoptClassInstance(workInProgress, instance);
204
return instance;
205
}