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

Es2015 classes #1734

Closed
wants to merge 3 commits into from
Closed

Es2015 classes #1734

wants to merge 3 commits into from

Conversation

cpojer
Copy link

@cpojer cpojer commented Sep 27, 2015

This is live from JSConf. Will explain later.

@oliviertassinari
Copy link
Member

@cpojer
Copy link
Author

cpojer commented Sep 27, 2015

Yes you are correct! Someone mentioned this project on Twitter when I asked for a big open source project that uses React.createClass. I tried to do a really cool and fun demo where I transform a lot of code automatically and then send out a live pull-request. People liked it and the video should be available soon.

I tested it locally using npm start in docs. It seems like travis uses a different copy of React and it doesn't load app.jsx. Let me know if you want this change (inlining PureRenderMixin is not necessarily awesome) or not. I'm happy to help adjust the transform to use patterns that work well for you. Alternatively, feel free to run this yourself: https://github.com/facebook/react/tree/master/packages/react-codemod :)

@oliviertassinari
Copy link
Member

I would refer to this issue facebook/react#613 (comment) regarding benefits and drawbacks of using ES6 classes.
Wow, great talk Christoph Pojer: Evolving Complex Systems Incrementally | JSConf EU 2015!

@cpojer
Copy link
Author

cpojer commented Oct 13, 2015

Thanks for building a project that I could use for my demo! :)

@shaurya947
Copy link
Contributor

Thanks @cpojer! This is a big thing for MUI :-)

Regarding the PR itself, I'm not sure whether we want to abandon the React.createClass syntax -- at least until we've replaced mixins with higher-order components. What do you guys think? cc: @oliviertassinari @hai-cea

class Master extends React.Component {
constructor(props, context) {
super(props, context);
this._handleTabChange = this._handleTabChange.bind(this);
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering. What is the advantage of using the bind operator?
This pattern seems to make the code maintainability harder.
Couldn't we use an arrow function or an @autobind?

(Related to #2439)

Copy link
Author

Choose a reason for hiding this comment

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

ES2015 class components don't use autobinding like React.createClass does. So you need to bind event handlers manually. If you want to use @autoBind or whatever other solution, the codemod can be adjusted and re-run to use that. Another solution we are hoping to make part of the ES2015 spec is to be able to define arrow functions as class properties, like this:

class A {
  onClick: () => this.foo();
} 

which would avoid the need for calling bind. But this hasn't been added to the next version of ES yet.

Copy link
Member

Choose a reason for hiding this comment

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

Ok thanks!
We are going to use the @autobind to beging with this migration.

@alitaheri alitaheri added this to the ES6 Classes milestone Dec 8, 2015
@alitaheri
Copy link
Member

I'm going to close this issue. thanks for letting us know this tool exists. we'll use it to migrate our codebase, thanks a lot 👍 👍

@alitaheri alitaheri closed this Dec 17, 2015
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Nov 10, 2020
Bumps [sinon](https://github.com/sinonjs/sinon) from 7.4.2 to 9.0.2.
- [Release notes](https://github.com/sinonjs/sinon/releases)
- [Changelog](https://github.com/sinonjs/sinon/blob/master/CHANGELOG.md)
- [Commits](sinonjs/sinon@v7.4.2...v9.0.2)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
@zannager zannager added the docs Improvements or additions to the documentation label Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants