Skip to content

Commit 847de6d

Browse files
author
Keyan Zhang
committed
Redesigned the way we start modding components.
Now we search and grab `React.createClass` _call expressions_ directly. The only time that we can't simply replace a `createClass` call path with a new class is when the parent of that is a variable declaration: `var Foo = React.createClass({...});` needs to be replaced entirely with `class Foo extends React.Component {...}` Now we delay checking it and only when we need to replace a path we take a look at `path.parentPath.value.type` to see if it's a variable declarator. With this commit we should be able to mod any kind of anonymous `createClass` calls no matter how they are defined.
1 parent 1011552 commit 847de6d

File tree

5 files changed

+222
-72
lines changed

5 files changed

+222
-72
lines changed
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* @flow
3+
*/
4+
/* eslint-disable no-use-before-define */
5+
'use strict';
6+
7+
var React = require('React');
8+
9+
var CrazyObject = {
10+
foo: {
11+
bar: 123,
12+
},
13+
method: {
14+
wrapThisGuy: (x) => x,
15+
deep: {
16+
wrapThatGuy: (x) => x,
17+
},
18+
},
19+
iDontUnderstand: {
20+
whyYouDoThis: {
21+
butAnyway: {
22+
comp1: React.createClass({
23+
render() {
24+
return <div/>;
25+
},
26+
}),
27+
comp2: CrazyObject.method.wrapThatGuy(React.createClass({
28+
render() {
29+
return <div/>;
30+
},
31+
})),
32+
waitWhatArrayForReal: [React.createClass({
33+
render() {
34+
return <div/>;
35+
},
36+
}), [React.createClass({
37+
render() {
38+
return <p/>;
39+
},
40+
}), React.createClass({
41+
render() {
42+
return <span/>;
43+
},
44+
})]],
45+
},
46+
},
47+
},
48+
};
49+
50+
module.exports = WaltUtils;
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* @flow
3+
*/
4+
/* eslint-disable no-use-before-define */
5+
'use strict';
6+
7+
var React = require('React');
8+
9+
var CrazyObject = {
10+
foo: {
11+
bar: 123,
12+
},
13+
method: {
14+
wrapThisGuy: (x) => x,
15+
deep: {
16+
wrapThatGuy: (x) => x,
17+
},
18+
},
19+
iDontUnderstand: {
20+
whyYouDoThis: {
21+
butAnyway: {
22+
comp1: class extends React.Component {
23+
render() {
24+
return <div/>;
25+
}
26+
},
27+
comp2: CrazyObject.method.wrapThatGuy(class extends React.Component {
28+
render() {
29+
return <div/>;
30+
}
31+
}),
32+
waitWhatArrayForReal: [class extends React.Component {
33+
render() {
34+
return <div/>;
35+
}
36+
}, [class extends React.Component {
37+
render() {
38+
return <p/>;
39+
}
40+
}, class extends React.Component {
41+
render() {
42+
return <span/>;
43+
}
44+
}]],
45+
},
46+
},
47+
},
48+
};
49+
50+
module.exports = WaltUtils;

transforms/__tests__/class-test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const enableFlowOption = {flow: true};
2121

2222
defineTest(__dirname, 'class');
2323
defineTest(__dirname, 'class', enableFlowOption, 'class-anonymous');
24+
defineTest(__dirname, 'class', enableFlowOption, 'class-anonymous2');
2425
defineTest(__dirname, 'class', pureMixinAlternativeOption, 'class-test2');
2526
defineTest(__dirname, 'class', enableFlowOption, 'export-default-class');
2627
defineTest(__dirname, 'class', pureMixinAlternativeOption, 'class-pure-mixin1');

transforms/class.js

Lines changed: 40 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ module.exports = (file, api, options) => {
162162
const hasNoCallsToDeprecatedAPIs = classPath => {
163163
if (checkDeprecatedAPICalls(classPath)) {
164164
console.warn(
165-
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
165+
file.path + ': `' + ReactUtils.directlyGetComponentName(classPath) + '` ' +
166166
'was skipped because of deprecated API calls. Remove calls to ' +
167167
DEPRECATED_APIS.join(', ') + ' in your React component and re-run ' +
168168
'this script.'
@@ -186,7 +186,7 @@ module.exports = (file, api, options) => {
186186

187187
if (hasInvalidCalls) {
188188
console.warn(
189-
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
189+
file.path + ': `' + ReactUtils.directlyGetComponentName(classPath) + '` ' +
190190
'was skipped because of API calls that will be removed. Remove calls to `' +
191191
DEFAULT_PROPS_FIELD + '` and/or `' + GET_INITIAL_STATE_FIELD +
192192
'` in your React component and re-run this script.'
@@ -202,7 +202,7 @@ module.exports = (file, api, options) => {
202202
);
203203
if (hasArguments) {
204204
console.warn(
205-
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
205+
file.path + ': `' + ReactUtils.directlyGetComponentName(classPath) + '` ' +
206206
'was skipped because `arguments` was found in your functions. ' +
207207
'Arrow functions do not expose an `arguments` object; ' +
208208
'consider changing to use ES6 spread operator and re-run this script.'
@@ -260,14 +260,14 @@ module.exports = (file, api, options) => {
260260
};
261261

262262
const isInitialStateConvertible = classPath => {
263-
const specPath = ReactUtils.getReactCreateClassSpec(classPath);
263+
const specPath = ReactUtils.directlyGetCreateClassSpec(classPath);
264264
if (!specPath) {
265265
return false;
266266
}
267267
const result = isGetInitialStateConstructorSafe(findGetInitialState(specPath));
268268
if (!result) {
269269
console.warn(
270-
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
270+
file.path + ': `' + ReactUtils.directlyGetComponentName(classPath) + '` ' +
271271
'was skipped because of potential shadowing issues were found in ' +
272272
'the React component. Rename variable declarations of `props` and/or `context` ' +
273273
'in your `getInitialState` and re-run this script.'
@@ -277,7 +277,7 @@ module.exports = (file, api, options) => {
277277
};
278278

279279
const canConvertToClass = classPath => {
280-
const specPath = ReactUtils.getReactCreateClassSpec(classPath);
280+
const specPath = ReactUtils.directlyGetCreateClassSpec(classPath);
281281
if (!specPath) {
282282
return false;
283283
}
@@ -299,7 +299,7 @@ module.exports = (file, api, options) => {
299299
.map(prop => prop.key.name ? prop.key.name : prop.key)
300300
.join(', ');
301301
console.warn(
302-
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
302+
file.path + ': `' + ReactUtils.directlyGetComponentName(classPath) + '` ' +
303303
'was skipped because of invalid field(s) `' + invalidText + '` on ' +
304304
'the React component. Remove any right-hand-side expressions that ' +
305305
'are not simple, like: `componentWillUpdate: createWillUpdate()` or ' +
@@ -311,8 +311,8 @@ module.exports = (file, api, options) => {
311311

312312
const areMixinsConvertible = (mixinIdentifierNames, classPath) => {
313313
if (
314-
ReactUtils.hasMixins(classPath) &&
315-
!ReactUtils.hasSpecificMixins(classPath, mixinIdentifierNames)
314+
ReactUtils.directlyHasMixinsField(classPath) &&
315+
!ReactUtils.directlyHasSpecificMixins(classPath, mixinIdentifierNames)
316316
) {
317317
return false;
318318
}
@@ -1023,34 +1023,40 @@ module.exports = (file, api, options) => {
10231023
);
10241024
});
10251025

1026-
const updateToClass = (classPath, type) => {
1027-
const specPath = ReactUtils.getReactCreateClassSpec(classPath);
1028-
const name = ReactUtils.getComponentName(classPath);
1026+
const updateToClass = (classPath) => {
1027+
const specPath = ReactUtils.directlyGetCreateClassSpec(classPath);
1028+
const name = ReactUtils.directlyGetComponentName(classPath);
10291029
const statics = collectStatics(specPath);
10301030
const properties = collectNonStaticProperties(specPath);
10311031
const comments = getComments(classPath);
10321032

10331033
const getInitialState = findGetInitialState(specPath);
10341034

1035-
var path;
1035+
var path = classPath;
1036+
10361037
if (
1037-
type == 'moduleExports' ||
1038-
type == 'exportDefault' ||
1039-
type == 'anonymousInCallExpression'
1038+
classPath.parentPath &&
1039+
classPath.parentPath.value &&
1040+
classPath.parentPath.value.type === 'VariableDeclarator'
10401041
) {
1041-
path = ReactUtils.findReactCreateClassCallExpression(classPath);
1042-
} else {
1043-
path = j(classPath).closest(j.VariableDeclaration);
1042+
// the reason that we need to do this awkward dance here is that
1043+
// for things like `var Foo = React.createClass({...})`, we need to
1044+
// replace the _entire_ VariableDeclaration with
1045+
// `class Foo extends React.Component {...}`.
1046+
// it looks scary but since we already know it's a VariableDeclarator
1047+
// it's actually safe.
1048+
// (VariableDeclaration > declarations > VariableDeclarator > CallExpression)
1049+
path = classPath.parentPath.parentPath.parentPath;
10441050
}
10451051

10461052
const staticProperties = createStaticClassProperties(statics);
10471053
const baseClassName =
10481054
pureRenderMixinPathAndBinding &&
1049-
ReactUtils.hasSpecificMixins(classPath, [pureRenderMixinPathAndBinding.binding]) ?
1055+
ReactUtils.directlyHasSpecificMixins(classPath, [pureRenderMixinPathAndBinding.binding]) ?
10501056
'PureComponent' :
10511057
'Component';
10521058

1053-
path.replaceWith(
1059+
j(path).replaceWith(
10541060
createESClass(
10551061
name,
10561062
baseClassName,
@@ -1071,7 +1077,7 @@ module.exports = (file, api, options) => {
10711077
// class mixins is an array and only contains the identifier -> true
10721078
// otherwise -> false
10731079
const mixinsFilter = (classPath) => {
1074-
if (!ReactUtils.hasMixins(classPath)) {
1080+
if (!ReactUtils.directlyHasMixinsField(classPath)) {
10751081
return true;
10761082
} else if (options['pure-component'] && pureRenderMixinPathAndBinding) {
10771083
const {binding} = pureRenderMixinPathAndBinding;
@@ -1080,32 +1086,30 @@ module.exports = (file, api, options) => {
10801086
}
10811087
}
10821088
console.warn(
1083-
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
1089+
file.path + ': `' + ReactUtils.directlyGetComponentName(classPath) + '` ' +
10841090
'was skipped because of inconvertible mixins.'
10851091
);
1092+
10861093
return false;
10871094
};
10881095

1089-
const apply = (path, type) =>
1096+
// the only time that we can't simply replace the createClass call path
1097+
// with a new class is when the parent of that is a variable declaration.
1098+
// let's delay it and figure it out later (by looking at `path.parentPath`)
1099+
// in `updateToClass`.
1100+
const apply = (path) =>
10901101
path
10911102
.filter(mixinsFilter)
10921103
.filter(hasNoCallsToDeprecatedAPIs)
10931104
.filter(hasNoRefsToAPIsThatWillBeRemoved)
10941105
.filter(doesNotUseArguments)
10951106
.filter(isInitialStateConvertible)
10961107
.filter(canConvertToClass)
1097-
.forEach(classPath => updateToClass(classPath, type));
1098-
1099-
const didTransform = (
1100-
apply(ReactUtils.findReactAnonymousCreateClassInCallExpression(root), 'anonymousInCallExpression')
1101-
.size() +
1102-
apply(ReactUtils.findReactCreateClass(root), 'var')
1103-
.size() +
1104-
apply(ReactUtils.findReactCreateClassModuleExports(root), 'moduleExports')
1105-
.size() +
1106-
apply(ReactUtils.findReactCreateClassExportDefault(root), 'exportDefault')
1107-
.size()
1108-
) > 0;
1108+
.forEach(updateToClass);
1109+
1110+
const didTransform = apply(
1111+
ReactUtils.findAllReactCreateClassCalls(root)
1112+
).size() > 0;
11091113

11101114
if (didTransform) {
11111115
// prune removed requires
@@ -1118,7 +1122,6 @@ module.exports = (file, api, options) => {
11181122

11191123
return root.toSource(printOptions);
11201124
}
1121-
11221125
}
11231126

11241127
return null;

0 commit comments

Comments
 (0)