diff --git a/transforms/__testfixtures__/class-anonymous2.input.js b/transforms/__testfixtures__/class-anonymous2.input.js new file mode 100644 index 00000000..a9024745 --- /dev/null +++ b/transforms/__testfixtures__/class-anonymous2.input.js @@ -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
; + }, + }), + comp2: CrazyObject.method.wrapThatGuy(React.createClass({ + render() { + return
; + }, + })), + waitWhatArrayForReal: [React.createClass({ + render() { + return
; + }, + }), [React.createClass({ + render() { + return

; + }, + }), React.createClass({ + render() { + return ; + }, + })]], + }, + }, + }, +}; + +module.exports = WaltUtils; diff --git a/transforms/__testfixtures__/class-anonymous2.output.js b/transforms/__testfixtures__/class-anonymous2.output.js new file mode 100644 index 00000000..1b7a8fcb --- /dev/null +++ b/transforms/__testfixtures__/class-anonymous2.output.js @@ -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

; + } + }, + comp2: CrazyObject.method.wrapThatGuy(class extends React.Component { + render() { + return
; + } + }), + waitWhatArrayForReal: [class extends React.Component { + render() { + return
; + } + }, [class extends React.Component { + render() { + return

; + } + }, class extends React.Component { + render() { + return ; + } + }]], + }, + }, + }, +}; + +module.exports = WaltUtils; diff --git a/transforms/__tests__/class-test.js b/transforms/__tests__/class-test.js index 4eb3f59b..b2a149b8 100644 --- a/transforms/__tests__/class-test.js +++ b/transforms/__tests__/class-test.js @@ -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'); diff --git a/transforms/class.js b/transforms/class.js index a008d068..6b76dd9e 100644 --- a/transforms/class.js +++ b/transforms/class.js @@ -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.' @@ -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.' @@ -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.' @@ -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.' @@ -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; } @@ -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 ' + @@ -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; } @@ -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, @@ -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; @@ -1080,13 +1086,18 @@ 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) @@ -1094,18 +1105,11 @@ module.exports = (file, api, options) => { .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 @@ -1118,7 +1122,6 @@ module.exports = (file, api, options) => { return root.toSource(printOptions); } - } return null; diff --git a/transforms/utils/ReactUtils.js b/transforms/utils/ReactUtils.js index 53498533..c09757a2 100644 --- a/transforms/utils/ReactUtils.js +++ b/transforms/utils/ReactUtils.js @@ -86,23 +86,12 @@ module.exports = function(j) { }, }); - const findReactAnonymousCreateClassInCallExpression = path => - path.find(j.CallExpression, { - arguments: [{ - type: 'CallExpression', - callee: REACT_CREATE_CLASS_MEMBER_EXPRESSION, - }], - }); - const getReactCreateClassSpec = classPath => { - var callCollection = findReactCreateClassCallExpression(classPath); - if (callCollection.size() === 0) { - return null; - } - const args = callCollection.get('arguments').value; - if (args) { + const {value} = classPath; + const args = (value.init || value.right || value.declaration).arguments; + if (args && args.length) { const spec = args[0]; - if (spec && spec.type === 'ObjectExpression' && Array.isArray(spec.properties)) { + if (spec.type === 'ObjectExpression' && Array.isArray(spec.properties)) { return spec; } } @@ -166,13 +155,47 @@ module.exports = function(j) { // --------------------------------------------------------------------------- // Checks if the React class has mixins - const isMixinProperty = property => property.key.name === 'mixins'; + const isMixinProperty = property => { + const key = property.key; + const value = property.value; + return ( + key.name === 'mixins' && + value.type === 'ArrayExpression' && + Array.isArray(value.elements) && + value.elements.length + ); + }; const hasMixins = classPath => { const spec = getReactCreateClassSpec(classPath); return spec && spec.properties.some(isMixinProperty); }; + // --------------------------------------------------------------------------- + // Others + const getClassExtendReactSpec = classPath => classPath.value.body; + + const createCreateReactClassCallExpression = properties => + j.callExpression( + j.memberExpression( + j.identifier('React'), + j.identifier('createClass'), + false + ), + [j.objectExpression(properties)] + ); + + const getComponentName = + classPath => classPath.node.id && classPath.node.id.name; + + // --------------------------------------------------------------------------- + // Direct methods! (see explanation below) + const findAllReactCreateClassCalls = path => + path.find(j.CallExpression, { + callee: REACT_CREATE_CLASS_MEMBER_EXPRESSION, + }); + + // Mixin Stuff const containSameElements = (ls1, ls2) => { if (ls1.length !== ls2.length) { return false; @@ -184,6 +207,8 @@ module.exports = function(j) { ); }; + const keyNameIsMixins = property => property.key.name === 'mixins'; + const isSpecificMixinsProperty = (property, mixinIdentifierNames) => { const key = property.key; const value = property.value; @@ -197,31 +222,46 @@ module.exports = function(j) { ); }; - const hasSpecificMixins = (classPath, mixinIdentifierNames) => { - const spec = getReactCreateClassSpec(classPath); - return spec && spec.properties.some(prop => isSpecificMixinsProperty(prop, mixinIdentifierNames)); + // These following methods assume that the argument is + // a `React.createClass` call expression. In other words, + // they should only be used with `findAllReactCreateClassCalls`. + const directlyGetCreateClassSpec = classPath => { + if (!classPath || !classPath.value) { + return null; + } + const args = classPath.value.arguments; + if (args && args.length) { + const spec = args[0]; + if (spec.type === 'ObjectExpression' && Array.isArray(spec.properties)) { + return spec; + } + } + return null; }; - // --------------------------------------------------------------------------- - // Others - const getClassExtendReactSpec = classPath => classPath.value.body; + const directlyGetComponentName = classPath => { + let result = ''; + if ( + classPath.parentPath.value && + classPath.parentPath.value.type === 'VariableDeclarator' + ) { + result = classPath.parentPath.value.id.name; + } + return result; + }; - const createCreateReactClassCallExpression = properties => - j.callExpression( - j.memberExpression( - j.identifier('React'), - j.identifier('createClass'), - false - ), - [j.objectExpression(properties)] - ); + const directlyHasMixinsField = classPath => { + const spec = directlyGetCreateClassSpec(classPath); + return spec && spec.properties.some(keyNameIsMixins); + }; - const getComponentName = - classPath => classPath.node.id && classPath.node.id.name; + const directlyHasSpecificMixins = (classPath, mixinIdentifierNames) => { + const spec = directlyGetCreateClassSpec(classPath); + return spec && spec.properties.some(prop => isSpecificMixinsProperty(prop, mixinIdentifierNames)); + }; return { createCreateReactClassCallExpression, - findReactAnonymousCreateClassInCallExpression, findReactES6ClassDeclaration, findReactCreateClass, findReactCreateClassCallExpression, @@ -231,9 +271,15 @@ module.exports = function(j) { getReactCreateClassSpec, getClassExtendReactSpec, hasMixins, - hasSpecificMixins, hasModule, hasReact, isMixinProperty, + + // "direct" methods + findAllReactCreateClassCalls, + directlyGetComponentName, + directlyGetCreateClassSpec, + directlyHasMixinsField, + directlyHasSpecificMixins, }; };