Skip to content

Commit

Permalink
Redesigned the way we start modding components.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Keyan Zhang committed Jul 6, 2016
1 parent 1011552 commit 847de6d
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 72 deletions.
50 changes: 50 additions & 0 deletions transforms/__testfixtures__/class-anonymous2.input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* @flow
*/
/* eslint-disable no-use-before-define */
'use strict';

var React = require('React');

var CrazyObject = {
foo: {
bar: 123,
},
method: {
wrapThisGuy: (x) => x,
deep: {
wrapThatGuy: (x) => x,
},
},
iDontUnderstand: {
whyYouDoThis: {
butAnyway: {
comp1: React.createClass({
render() {
return <div/>;
},
}),
comp2: CrazyObject.method.wrapThatGuy(React.createClass({
render() {
return <div/>;
},
})),
waitWhatArrayForReal: [React.createClass({
render() {
return <div/>;
},
}), [React.createClass({
render() {
return <p/>;
},
}), React.createClass({
render() {
return <span/>;
},
})]],
},
},
},
};

module.exports = WaltUtils;
50 changes: 50 additions & 0 deletions transforms/__testfixtures__/class-anonymous2.output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* @flow
*/
/* eslint-disable no-use-before-define */
'use strict';

var React = require('React');

var CrazyObject = {
foo: {
bar: 123,
},
method: {
wrapThisGuy: (x) => x,
deep: {
wrapThatGuy: (x) => x,
},
},
iDontUnderstand: {
whyYouDoThis: {
butAnyway: {
comp1: class extends React.Component {
render() {
return <div/>;
}
},
comp2: CrazyObject.method.wrapThatGuy(class extends React.Component {
render() {
return <div/>;
}
}),
waitWhatArrayForReal: [class extends React.Component {
render() {
return <div/>;
}
}, [class extends React.Component {
render() {
return <p/>;
}
}, class extends React.Component {
render() {
return <span/>;
}
}]],
},
},
},
};

module.exports = WaltUtils;
1 change: 1 addition & 0 deletions transforms/__tests__/class-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const enableFlowOption = {flow: true};

defineTest(__dirname, 'class');
defineTest(__dirname, 'class', enableFlowOption, 'class-anonymous');
defineTest(__dirname, 'class', enableFlowOption, 'class-anonymous2');
defineTest(__dirname, 'class', pureMixinAlternativeOption, 'class-test2');
defineTest(__dirname, 'class', enableFlowOption, 'export-default-class');
defineTest(__dirname, 'class', pureMixinAlternativeOption, 'class-pure-mixin1');
Expand Down
77 changes: 40 additions & 37 deletions transforms/class.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ module.exports = (file, api, options) => {
const hasNoCallsToDeprecatedAPIs = classPath => {
if (checkDeprecatedAPICalls(classPath)) {
console.warn(
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
file.path + ': `' + ReactUtils.directlyGetComponentName(classPath) + '` ' +
'was skipped because of deprecated API calls. Remove calls to ' +
DEPRECATED_APIS.join(', ') + ' in your React component and re-run ' +
'this script.'
Expand All @@ -186,7 +186,7 @@ module.exports = (file, api, options) => {

if (hasInvalidCalls) {
console.warn(
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
file.path + ': `' + ReactUtils.directlyGetComponentName(classPath) + '` ' +
'was skipped because of API calls that will be removed. Remove calls to `' +
DEFAULT_PROPS_FIELD + '` and/or `' + GET_INITIAL_STATE_FIELD +
'` in your React component and re-run this script.'
Expand All @@ -202,7 +202,7 @@ module.exports = (file, api, options) => {
);
if (hasArguments) {
console.warn(
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
file.path + ': `' + ReactUtils.directlyGetComponentName(classPath) + '` ' +
'was skipped because `arguments` was found in your functions. ' +
'Arrow functions do not expose an `arguments` object; ' +
'consider changing to use ES6 spread operator and re-run this script.'
Expand Down Expand Up @@ -260,14 +260,14 @@ module.exports = (file, api, options) => {
};

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

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

const areMixinsConvertible = (mixinIdentifierNames, classPath) => {
if (
ReactUtils.hasMixins(classPath) &&
!ReactUtils.hasSpecificMixins(classPath, mixinIdentifierNames)
ReactUtils.directlyHasMixinsField(classPath) &&
!ReactUtils.directlyHasSpecificMixins(classPath, mixinIdentifierNames)
) {
return false;
}
Expand Down Expand Up @@ -1023,34 +1023,40 @@ module.exports = (file, api, options) => {
);
});

const updateToClass = (classPath, type) => {
const specPath = ReactUtils.getReactCreateClassSpec(classPath);
const name = ReactUtils.getComponentName(classPath);
const updateToClass = (classPath) => {
const specPath = ReactUtils.directlyGetCreateClassSpec(classPath);
const name = ReactUtils.directlyGetComponentName(classPath);
const statics = collectStatics(specPath);
const properties = collectNonStaticProperties(specPath);
const comments = getComments(classPath);

const getInitialState = findGetInitialState(specPath);

var path;
var path = classPath;

if (
type == 'moduleExports' ||
type == 'exportDefault' ||
type == 'anonymousInCallExpression'
classPath.parentPath &&
classPath.parentPath.value &&
classPath.parentPath.value.type === 'VariableDeclarator'
) {
path = ReactUtils.findReactCreateClassCallExpression(classPath);
} else {
path = j(classPath).closest(j.VariableDeclaration);
// the reason that we need to do this awkward dance here is that
// for things like `var Foo = React.createClass({...})`, we need to
// replace the _entire_ VariableDeclaration with
// `class Foo extends React.Component {...}`.
// it looks scary but since we already know it's a VariableDeclarator
// it's actually safe.
// (VariableDeclaration > declarations > VariableDeclarator > CallExpression)
path = classPath.parentPath.parentPath.parentPath;
}

const staticProperties = createStaticClassProperties(statics);
const baseClassName =
pureRenderMixinPathAndBinding &&
ReactUtils.hasSpecificMixins(classPath, [pureRenderMixinPathAndBinding.binding]) ?
ReactUtils.directlyHasSpecificMixins(classPath, [pureRenderMixinPathAndBinding.binding]) ?
'PureComponent' :
'Component';

path.replaceWith(
j(path).replaceWith(
createESClass(
name,
baseClassName,
Expand All @@ -1071,7 +1077,7 @@ module.exports = (file, api, options) => {
// class mixins is an array and only contains the identifier -> true
// otherwise -> false
const mixinsFilter = (classPath) => {
if (!ReactUtils.hasMixins(classPath)) {
if (!ReactUtils.directlyHasMixinsField(classPath)) {
return true;
} else if (options['pure-component'] && pureRenderMixinPathAndBinding) {
const {binding} = pureRenderMixinPathAndBinding;
Expand All @@ -1080,32 +1086,30 @@ module.exports = (file, api, options) => {
}
}
console.warn(
file.path + ': `' + ReactUtils.getComponentName(classPath) + '` ' +
file.path + ': `' + ReactUtils.directlyGetComponentName(classPath) + '` ' +
'was skipped because of inconvertible mixins.'
);

return false;
};

const apply = (path, type) =>
// the only time that we can't simply replace the createClass call path
// with a new class is when the parent of that is a variable declaration.
// let's delay it and figure it out later (by looking at `path.parentPath`)
// in `updateToClass`.
const apply = (path) =>
path
.filter(mixinsFilter)
.filter(hasNoCallsToDeprecatedAPIs)
.filter(hasNoRefsToAPIsThatWillBeRemoved)
.filter(doesNotUseArguments)
.filter(isInitialStateConvertible)
.filter(canConvertToClass)
.forEach(classPath => updateToClass(classPath, type));

const didTransform = (
apply(ReactUtils.findReactAnonymousCreateClassInCallExpression(root), 'anonymousInCallExpression')
.size() +
apply(ReactUtils.findReactCreateClass(root), 'var')
.size() +
apply(ReactUtils.findReactCreateClassModuleExports(root), 'moduleExports')
.size() +
apply(ReactUtils.findReactCreateClassExportDefault(root), 'exportDefault')
.size()
) > 0;
.forEach(updateToClass);

const didTransform = apply(
ReactUtils.findAllReactCreateClassCalls(root)
).size() > 0;

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

return root.toSource(printOptions);
}

}

return null;
Expand Down
Loading

0 comments on commit 847de6d

Please sign in to comment.