Change class property transform to use rest params/spread operator

This allows the class property functions to be hot-reloaded properly when changing the number of arguments
GlenTiki · Jul 27, 2016 · caf6d51 · caf6d51
1 parent f474b69
commit caf6d51
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 30 deletions.
diff --git a/src/babel/index.js b/src/babel/index.js
@@ -14,25 +14,17 @@ const buildTagger = template(`
 })();
 `);
 
-const cleanUpParams = (t, params) => (
-  params.map((param, i) => {
-    if (param.type === 'AssignmentPattern') {
-      return param.left;
-    } else if (param.type === 'ObjectPattern') {
-      return t.identifier(`param${i}`);
-    }
-    return param;
-  })
-);
-
-const buildNewClassProperty = (t, key, identifier, params) => {
-  const returnStatement = t.returnStatement(t.callExpression(
-    t.memberExpression(t.thisExpression(), identifier),
-    params
-  ));
-  const arrowFunctionBody = t.blockStatement([returnStatement]);
-  const newArrowFunction = t.arrowFunctionExpression(params, arrowFunctionBody);
-  return t.classProperty(key, newArrowFunction);
+const buildNewClassProperty = (t, classPropertyName, newMethodName) => {
+  const returnExpression = t.callExpression(
+    t.memberExpression(t.thisExpression(), newMethodName),
+    [t.spreadElement(t.identifier('params'))]
+  );
+
+  const newArrowFunction = t.arrowFunctionExpression(
+    [t.restElement(t.identifier('params'))],
+    returnExpression
+  );
+  return t.classProperty(classPropertyName, newArrowFunction);
 };
 
 module.exports = function plugin(args) {
@@ -171,9 +163,7 @@ module.exports = function plugin(args) {
 
               // replace the original class property function with a function that calls
               // the new class method created above
-              path.replaceWith(
-                buildNewClassProperty(t, node.key, newIdentifier, cleanUpParams(t, params))
-              );
+              path.replaceWith(buildNewClassProperty(t, node.key, newIdentifier));
             }
           }
         });

diff --git a/test/AppContainer/AppContainer.dev.js b/test/AppContainer/AppContainer.dev.js
@@ -336,6 +336,56 @@ function runAllTests(useWeakMap) {
         wrapper.find('span').simulate('click');
         expect(spy).toHaveBeenCalledWith('bar');
       });
+
+      it('replaces children with class property arrow ' +
+         'functions with different numbers of arguments', () => {
+        const spy = createSpy();
+
+        class App extends Component {
+          componentWillMount() {
+            this.state = 'new';
+          }
+
+          shouldComponentUpdate() {
+            return false;
+          }
+
+          handleClick = () => spy('foo');
+
+          render() {
+            return <span onClick={this.handleClick}>old render</span>;
+          }
+        }
+        RHL.register(App, 'App', 'test.js');
+
+        const wrapper = mount(<AppContainer><App /></AppContainer>);
+        wrapper.find('span').simulate('click');
+        expect(spy).toHaveBeenCalledWith('foo');
+
+        spy.reset();
+        {
+          class App extends Component {
+            componentWillMount() {
+              this.state = 'new';
+            }
+
+            shouldComponentUpdate() {
+              return false;
+            }
+
+            handleClick = ({ target }) => spy(target.value);
+
+            render() {
+              return <span onClick={this.handleClick}>new render</span>;
+            }
+          }
+          RHL.register(App, 'App', 'test.js');
+          wrapper.setProps({ children: <App /> });
+        }
+
+        wrapper.find('span').simulate('click', { target: { value: 'bar' } });
+        expect(spy).toHaveBeenCalledWith('bar');
+      });
     });
 
     describe('with createClass root', () => {

diff --git a/test/babel/fixtures/class-properties/block-body/expected.js b/test/babel/fixtures/class-properties/block-body/expected.js
@@ -10,8 +10,8 @@ var Foo = function () {
 
     _classCallCheck(this, Foo);
 
-    this.bar = function (a, b) {
-      return _this.__bar__REACT_HOT_LOADER__(a, b);
+    this.bar = function () {
+      return _this.__bar__REACT_HOT_LOADER__.apply(_this, arguments);
     };
   }
 

diff --git a/test/babel/fixtures/class-properties/default-params/expected.js b/test/babel/fixtures/class-properties/default-params/expected.js
@@ -10,8 +10,8 @@ var Foo = function () {
 
     _classCallCheck(this, Foo);
 
-    this.bar = function (a) {
-      return _this.__bar__REACT_HOT_LOADER__(a);
+    this.bar = function () {
+      return _this.__bar__REACT_HOT_LOADER__.apply(_this, arguments);
     };
   }
 

diff --git a/test/babel/fixtures/class-properties/destructured-params/expected.js b/test/babel/fixtures/class-properties/destructured-params/expected.js
@@ -10,8 +10,8 @@ var Foo = function () {
 
     _classCallCheck(this, Foo);
 
-    this.bar = function (param0, param1) {
-      return _this.__bar__REACT_HOT_LOADER__(param0, param1);
+    this.bar = function () {
+      return _this.__bar__REACT_HOT_LOADER__.apply(_this, arguments);
     };
   }
 

diff --git a/test/babel/fixtures/class-properties/expression-body/expected.js b/test/babel/fixtures/class-properties/expression-body/expected.js
@@ -10,8 +10,8 @@ var Foo = function () {
 
     _classCallCheck(this, Foo);
 
-    this.onClick = function (e) {
-      return _this.__onClick__REACT_HOT_LOADER__(e);
+    this.onClick = function () {
+      return _this.__onClick__REACT_HOT_LOADER__.apply(_this, arguments);
     };
   }