Skip to content

Commit d67b6a8

Browse files
author
Keyan Zhang
committed
no shadowing in constructor
1 parent 4f64cc2 commit d67b6a8

File tree

3 files changed

+117
-0
lines changed

3 files changed

+117
-0
lines changed

transforms/__testfixtures__/class-initial-state.input.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,3 +180,26 @@ var UseArguments = React.createClass({
180180
return null;
181181
},
182182
});
183+
184+
var ShadowingIssue = React.createClass({
185+
getInitialState() {
186+
const props = { x: 123 };
187+
return { x: props.x };
188+
},
189+
190+
render() {
191+
return null;
192+
},
193+
});
194+
195+
var ShadowingButFine = React.createClass({
196+
getInitialState() {
197+
const props = this.props;
198+
const context = this.context;
199+
return { x: props.x + context.x };
200+
},
201+
202+
render() {
203+
return null;
204+
},
205+
});

transforms/__testfixtures__/class-initial-state.output.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,3 +189,25 @@ var UseArguments = React.createClass({
189189
return null;
190190
},
191191
});
192+
193+
var ShadowingIssue = React.createClass({
194+
getInitialState() {
195+
const props = { x: 123 };
196+
return { x: props.x };
197+
},
198+
199+
render() {
200+
return null;
201+
},
202+
});
203+
204+
class ShadowingButFine extends React.Component {
205+
constructor(props, context) {
206+
super(props, context);
207+
this.state = { x: props.x + context.x };
208+
}
209+
210+
render() {
211+
return null;
212+
}
213+
}

transforms/class.js

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,68 @@ module.exports = (file, api, options) => {
130130
return true;
131131
};
132132

133+
const isGetInitialStateConstructorSafe = getInitialState => {
134+
if (!getInitialState) {
135+
return true;
136+
}
137+
138+
const collection = j(getInitialState);
139+
let result = true;
140+
141+
const propsVarDeclarationCount = collection.find(j.VariableDeclarator, {
142+
id: {name: 'props'},
143+
}).size();
144+
145+
const contextVarDeclarationCount = collection.find(j.VariableDeclarator, {
146+
id: {name: 'context'},
147+
}).size();
148+
149+
if (
150+
propsVarDeclarationCount &&
151+
propsVarDeclarationCount !== collection.find(j.VariableDeclarator, {
152+
id: {name: 'props'},
153+
init: {
154+
type: 'MemberExpression',
155+
object: {type: 'ThisExpression'},
156+
property: {name: 'props'},
157+
}
158+
}).size()
159+
) {
160+
result = false;
161+
}
162+
163+
if (
164+
contextVarDeclarationCount &&
165+
contextVarDeclarationCount !== collection.find(j.VariableDeclarator, {
166+
id: {name: 'context'},
167+
init: {
168+
type: 'MemberExpression',
169+
object: {type: 'ThisExpression'},
170+
property: {name: 'context'},
171+
}
172+
}).size()
173+
) {
174+
result = false;
175+
}
176+
177+
return result;
178+
};
179+
180+
const isInitialStateConvertible = classPath => {
181+
const result = isGetInitialStateConstructorSafe(
182+
ReactUtils.getReactCreateClassSpec(classPath)
183+
);
184+
if (!result) {
185+
console.warn(
186+
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
187+
'was skipped because of potential shadowing issues were found in ' +
188+
'the React component. Rename variable declarations of `props` and/or `context` ' +
189+
'in your `getInitialState` and re-run this script.'
190+
);
191+
}
192+
return result;
193+
};
194+
133195
const canConvertToClass = classPath => {
134196
const specPath = ReactUtils.getReactCreateClassSpec(classPath);
135197
const invalidProperties = specPath.properties.filter(prop => (
@@ -323,6 +385,15 @@ module.exports = (file, api, options) => {
323385
const inlineGetInitialState = getInitialState => {
324386
const functionExpressionAST = j(getInitialState.value);
325387

388+
// at this point if there exists bindings like `const props = ...`, we
389+
// already know the RHS must be `this.props` (see `isGetInitialStateConstructorSafe`)
390+
// so it's safe to just remove them
391+
functionExpressionAST.find(j.VariableDeclarator, {id: {name: 'props'}})
392+
.forEach(path => j(path).remove());
393+
394+
functionExpressionAST.find(j.VariableDeclarator, {id: {name: 'context'}})
395+
.forEach(path => j(path).remove());
396+
326397
return functionExpressionAST
327398
.find(j.ReturnStatement)
328399
.forEach(path => {
@@ -880,6 +951,7 @@ module.exports = (file, api, options) => {
880951
.filter(hasNoCallsToDeprecatedAPIs)
881952
.filter(hasNoCallsToAPIsThatWillBeRemoved)
882953
.filter(doesNotUseArguments)
954+
.filter(isInitialStateConvertible)
883955
.filter(canConvertToClass)
884956
.forEach(classPath => updateToClass(classPath, type));
885957

0 commit comments

Comments
 (0)