Fix pure component transform edge cases (#255)

* Lexical declarations cannot be combined with the default keyword

* Remove unused super class import

* Don't include props argument if not used

Author: wesleyyee

transforms/__testfixtures__/pure-component.input.js

@@ -31,6 +31,12 @@ class ImpureWithRef extends React.Component {
   }
 }
 
+class PureWithoutProps extends React.Component {
+  render() {
+    return <div />;
+  }
+}
+
 class PureWithTypes extends React.Component {
   props: { foo: string };
   render() {
@@ -65,15 +71,15 @@ class PureWithPropTypes extends React.Component {
   static propTypes = { foo: React.PropTypes.string };
   static foo = 'bar';
   render() {
-    return <div />;
+    return <div>{this.props.foo}</div>;
   }
 }
 
 class PureWithPropTypes2 extends React.Component {
   props: { foo: string };
   static propTypes = { foo: React.PropTypes.string };
   render() {
-    return <div />;
+    return <div>{this.props.foo}</div>;
   }
 }
 

transforms/__testfixtures__/pure-component.output.js

@@ -29,6 +29,10 @@ class ImpureWithRef extends React.Component {
   }
 }
 
+function PureWithoutProps() {
+  return <div />;
+}
+
 function PureWithTypes(props: { foo: string }) {
   return <div className={props.foo} />;
 }
@@ -54,14 +58,14 @@ class ImpureClassPropertyWithTypes extends React.Component {
 }
 
 function PureWithPropTypes(props) {
-  return <div />;
+  return <div>{props.foo}</div>;
 }
 
 PureWithPropTypes.propTypes = { foo: React.PropTypes.string };
 PureWithPropTypes.foo = 'bar';
 
 function PureWithPropTypes2(props: { foo: string }) {
-  return <div />;
+  return <div>{props.foo}</div>;
 }
 
 PureWithPropTypes2.propTypes = { foo: React.PropTypes.string };

transforms/__testfixtures__/pure-component2.input.js

@@ -21,6 +21,12 @@ class Impure extends React.Component {
   }
 }
 
+class PureWithoutProps extends React.Component {
+  render() {
+    return <div />;
+  }
+}
+
 class PureWithTypes extends React.Component {
   props: { foo: string };
   render() {
@@ -40,7 +46,7 @@ class PureWithTypes2 extends React.Component {
 class PureWithPropTypes extends React.Component {
   static propTypes = { foo: React.PropTypes.string };
   render() {
-    return <div />;
+    return <div>{this.props.foo}</div>;
   }
 }
 

transforms/__testfixtures__/pure-component2.output.js

@@ -19,6 +19,10 @@ class Impure extends React.Component {
   }
 }
 
+const PureWithoutProps = () => {
+  return <div />;
+};
+
 const PureWithTypes = (props: { foo: string }) => {
   return <div className={props.foo} />;
 };
@@ -30,7 +34,7 @@ const PureWithTypes2 = (props: Props) => {
 };
 
 const PureWithPropTypes = props => {
-  return <div />;
+  return <div>{props.foo}</div>;
 };
 
 PureWithPropTypes.propTypes = { foo: React.PropTypes.string };

transforms/pure-component.js

@@ -202,16 +202,29 @@ module.exports = function(file, api, options) {
     return property && property.typeAnnotation.typeAnnotation;
   };
 
-  const build = useArrows => (name, body, typeAnnotation, destructure) => {
+  const isDefaultExport = path =>
+    path.parentPath && path.parentPath.value.type === 'ExportDefaultDeclaration';
+
+  const safelyDefaultExportDeclaration = (path) => {
+    const localName = path.value.declarations[0].id.name;
+    j(path.parent)
+      .replaceWith(_ => path.value)
+      .insertAfter(
+        j.exportDeclaration(true, { type: 'Identifier', name: localName })
+      );
+  };
+
+  const build = useArrows => (name, body, typeAnnotation, destructure, hasThisDotProps) => {
     const identifier = j.identifier(name);
     const propsIdentifier = buildIdentifierWithTypeAnnotation(
       'props',
       typeAnnotation
     );
-    const propsArg = [
+
+    const propsArg = hasThisDotProps ? [
       (destructure && destructureProps(j(body), typeAnnotation)) ||
         propsIdentifier
-    ];
+    ] : [];
     if (useArrows) {
       return j.variableDeclaration('const', [
         j.variableDeclarator(
@@ -261,6 +274,7 @@ module.exports = function(file, api, options) {
       if (!isPure && !silenceWarnings) {
         reportSkipped(path);
       }
+
       return isPure;
     }
   );
@@ -269,6 +283,10 @@ module.exports = function(file, api, options) {
     return null;
   }
 
+  // Save the names of the deleted pure classes super class
+  // We need this to prune unused variables at the end.
+  const parentClassNames = pureClasses.nodes().map(node => node.superClass.name);
+
   pureClasses.replaceWith(p => {
     const name = p.node.id.name;
     const renderMethod = p.value.body.body.filter(isRenderMethod)[0];
@@ -281,6 +299,7 @@ module.exports = function(file, api, options) {
       console.warn(`Unable to destructure ${name} props.`);
     }
 
+    const hasThisDotProps = j(renderBody).find(j.MemberExpression, THIS_PROPS).length > 0;
     replaceThisProps(renderBody);
 
     if (useArrows) {
@@ -289,7 +308,8 @@ module.exports = function(file, api, options) {
           name,
           renderBody,
           propsTypeAnnotation,
-          destructure
+          destructure,
+          hasThisDotProps
         ),
         ...buildStatics(name, statics)
       ];
@@ -299,11 +319,20 @@ module.exports = function(file, api, options) {
           name,
           renderBody,
           propsTypeAnnotation,
-          destructure
+          destructure,
+          hasThisDotProps
         ),
         ...buildStatics(name, statics)
       ];
     }
+  }).forEach(p => {
+    // Check for combining default keyword with const declaration
+    if (useArrows && isDefaultExport(p)) {
+      safelyDefaultExportDeclaration(p);
+    }
+  }).forEach((p, i) => {
+    const parentClassName = parentClassNames[i];
+    ReactUtils.removeUnusedSuperClassImport(j(p), f, parentClassName);
   });
 
   return f.toSource(printOptions);

transforms/utils/ReactUtils.js

@@ -123,6 +123,20 @@ module.exports = function(j) {
       : undefined;
   };
 
+  const removeUnusedSuperClassImport = (path, file, superClassName) => {
+    if (path.find(j.Identifier, {
+      type: 'Identifier',
+      name: superClassName
+    }).length === 0) {
+      file.find(j.ImportSpecifier, {
+        type: 'ImportSpecifier',
+        imported: {
+          type: 'Identifier',
+          name: superClassName,
+        }
+      }).remove();
+    }
+  };
 
   const findReactES6ClassDeclarationByParent = (path, parentClassName) => {
     const componentImport = findReactComponentNameByParent(path, parentClassName);
@@ -282,6 +296,7 @@ module.exports = function(j) {
     hasModule,
     hasReact,
     isMixinProperty,
+    removeUnusedSuperClassImport,
 
     // "direct" methods
     findAllReactCreateClassCalls,