Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add transform to support arrow function class properties #322

Merged
merged 14 commits into from
Sep 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .babelrc
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"presets": ["es2015", "react"]
}
"presets": ["es2015", "react", "stage-2"]
}
6 changes: 1 addition & 5 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
{
"extends": "airbnb",
"parserOptions": {
"ecmaFeatures": {
"experimentalObjectRestSpread": true
},
},
"parser": "babel-eslint",
"rules": {
"no-underscore-dangle": ["error", { "allow": ["__REACT_HOT_LOADER__"] }],
"no-console": ["error", { allow: ["error"] }],
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@
"devDependencies": {
"babel-cli": "^6.7.5",
"babel-core": "^6.7.6",
"babel-eslint": "^6.1.2",
"babel-preset-es2015": "^6.6.0",
"babel-preset-react": "^6.5.0",
"babel-preset-stage-2": "^6.5.0",
"enzyme": "^2.2.0",
"eslint": "^2.9.0",
"eslint-config-airbnb": "^8.0.0",
Expand Down
71 changes: 71 additions & 0 deletions src/babel/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,37 @@ const buildTagger = template(`
})();
`);

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);
};

const classPropertyOptOutVistor = {
MetaProperty(path, state) {
const { node } = path;

if (node.meta.name === 'new' && node.property.name === 'target') {
state.optOut = true; // eslint-disable-line no-param-reassign
}
},

ReferencedIdentifier(path, state) {
const { node } = path;

if (node.name === 'arguments') {
state.optOut = true; // eslint-disable-line no-param-reassign
}
},
};

module.exports = function plugin(args) {
// This is a Babel plugin, but the user put it in the Webpack config.
if (this && this.callback) {
Expand Down Expand Up @@ -125,6 +156,46 @@ module.exports = function plugin(args) {
node.body.push(buildSemi());
},
},

Class(classPath) {
const classBody = classPath.get('body');

classBody.get('body').forEach(path => {
if (path.isClassProperty()) {
const { node } = path;

const state = {
optOut: false,
};

path.traverse(classPropertyOptOutVistor, state);

if (state.optOut) {
return;
}

// class property node value is nullable
if (node.value && node.value.type === 'ArrowFunctionExpression') {
const params = node.value.params;
const newIdentifier = t.identifier(`__${node.key.name}__REACT_HOT_LOADER__`);

// arrow function body can either be a block statement or a returned expression
const newMethodBody = node.value.body.type === 'BlockStatement' ?
node.value.body :
t.blockStatement([t.returnStatement(node.value.body)]);

// create a new method on the class that the original class property function
// calls, since the method is able to be replaced by RHL
const newMethod = t.classMethod('method', newIdentifier, params, newMethodBody);
path.insertAfter(newMethod);

// replace the original class property function with a function that calls
// the new class method created above
path.replaceWith(buildNewClassProperty(t, node.key, newIdentifier));
}
}
});
},
},
};
};
4 changes: 4 additions & 0 deletions test/AppContainer/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"presets": ["es2015", "stage-2", "react"],
"plugins": ["../../src/babel"]
}
229 changes: 229 additions & 0 deletions test/AppContainer/AppContainer.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,235 @@ function runAllTests(useWeakMap) {

expect(wrapper.text()).toBe('new render + old state');
});

it('replaces children class methods', () => {
const spy = createSpy();

class App extends Component {
componentWillMount() {
this.state = 'old';
}

shouldComponentUpdate() {
return false;
}

handleClick() {
spy('foo');
}

render() {
return (
<span onClick={this.handleClick}>old render + {this.state} state</span>
);
}
}
RHL.register(App, 'App', 'test.js');

const wrapper = mount(<AppContainer><App /></AppContainer>);
wrapper.find('span').simulate('click');
expect(spy).toHaveBeenCalledWith('foo');
expect(wrapper.text()).toBe('old render + old state');

spy.reset();
{
class App extends Component {
componentWillMount() {
this.state = 'new';
}

shouldComponentUpdate() {
return false;
}

handleClick() {
spy('bar');
}

render() {
return (
<span onClick={this.handleClick}>new render + {this.state} state</span>
);
}
}
RHL.register(App, 'App', 'test.js');
wrapper.setProps({ children: <App /> });
}

wrapper.find('span').simulate('click');
expect(spy).toHaveBeenCalledWith('bar');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@calesce it might be worth it to test that the method is replaced without remounting the component, eg. like it is done here https://github.com/gaearon/react-hot-loader/blob/next/test/AppContainer/AppContainer.dev.js#L182

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, doesn't hurt.

expect(wrapper.text()).toBe('new render + old state');
});

it('replaces children class property arrow functions', () => {
const spy = createSpy();

class App extends Component {
componentWillMount() {
this.state = 'old';
}

shouldComponentUpdate() {
return false;
}

handleClick = () => {
spy('foo');
};

render() {
return (
<span onClick={this.handleClick}>old render + {this.state} state</span>
);
}
}
RHL.register(App, 'App', 'test.js');

const wrapper = mount(<AppContainer><App /></AppContainer>);
wrapper.find('span').simulate('click');
expect(spy).toHaveBeenCalledWith('foo');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above about testing that it doesnt remount

expect(wrapper.text()).toBe('old render + old state');

spy.reset();
{
class App extends Component {
componentWillMount() {
this.state = 'new';
}

shouldComponentUpdate() {
return false;
}

handleClick = () => {
spy('bar');
};

render() {
return (
<span onClick={this.handleClick}>new render + {this.state} state</span>
);
}
}
RHL.register(App, 'App', 'test.js');
wrapper.setProps({ children: <App /> });
}

wrapper.find('span').simulate('click');
expect(spy).toHaveBeenCalledWith('bar');
expect(wrapper.text()).toBe('new render + old state');
});

it('replaces children class property arrow functions without block statement bodies', () => {
const spy = createSpy();

class App extends Component {
componentWillMount() {
this.state = 'old';
}

shouldComponentUpdate() {
return false;
}

handleClick = () => spy('foo');

render() {
return (
<span onClick={this.handleClick}>old render + {this.state} state</span>
);
}
}
RHL.register(App, 'App', 'test.js');

const wrapper = mount(<AppContainer><App /></AppContainer>);
wrapper.find('span').simulate('click');
expect(spy).toHaveBeenCalledWith('foo');
expect(wrapper.text()).toBe('old render + old state');

spy.reset();
{
class App extends Component {
componentWillMount() {
this.state = 'new';
}

shouldComponentUpdate() {
return false;
}

handleClick = () => spy('bar');

render() {
return (
<span onClick={this.handleClick}>new render + {this.state} state</span>
);
}
}
RHL.register(App, 'App', 'test.js');
wrapper.setProps({ children: <App /> });
}

wrapper.find('span').simulate('click');
expect(spy).toHaveBeenCalledWith('bar');
expect(wrapper.text()).toBe('new render + old state');
});

it('replaces children with class property arrow ' +
'functions with different numbers of arguments', () => {
const spy = createSpy();

class App extends Component {
componentWillMount() {
this.state = 'old';
}

shouldComponentUpdate() {
return false;
}

handleClick = () => spy('foo');

render() {
return (
<span onClick={this.handleClick}>old render + {this.state} state</span>
);
}
}
RHL.register(App, 'App', 'test.js');

const wrapper = mount(<AppContainer><App /></AppContainer>);
wrapper.find('span').simulate('click');
expect(spy).toHaveBeenCalledWith('foo');
expect(wrapper.text()).toBe('old render + old state');

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 + {this.state} state</span>
);
}
}
RHL.register(App, 'App', 'test.js');
wrapper.setProps({ children: <App /> });
}

wrapper.find('span').simulate('click', { target: { value: 'bar' } });
expect(spy).toHaveBeenCalledWith('bar');
expect(wrapper.text()).toBe('new render + old state');
});
});

describe('with createClass root', () => {
Expand Down
3 changes: 3 additions & 0 deletions test/babel/fixtures/class-properties/arguments/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"plugins": ["syntax-class-properties", "../../../../../src/babel"]
}
7 changes: 7 additions & 0 deletions test/babel/fixtures/class-properties/arguments/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class Foo {
bar = (a, b) => {
arguments;

return a(b);
};
}
19 changes: 19 additions & 0 deletions test/babel/fixtures/class-properties/arguments/expected.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
var _arguments = arguments;
class Foo {
bar = (a, b) => {
_arguments;

return a(b);
};
}
;

(function () {
if (typeof __REACT_HOT_LOADER__ === 'undefined') {
return;
}

__REACT_HOT_LOADER__.register(Foo, "Foo", __FILENAME__);
})();

;
3 changes: 3 additions & 0 deletions test/babel/fixtures/class-properties/block-body/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"plugins": ["syntax-class-properties", "../../../../../src/babel"]
}
5 changes: 5 additions & 0 deletions test/babel/fixtures/class-properties/block-body/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class Foo {
bar = (a, b) => {
return a(b);
};
}
Loading