reactjs / core-notes

Weekly meeting notes from the React core team
900 stars 47 forks source link

Add May 19 notes #15

Closed gaearon closed 8 years ago

gaearon commented 8 years ago

Feel free to discuss here!

gerryster commented 8 years ago

I am a huge fan of these weekly minutes as they are a great way to stay on top of where the React team is headed. Thanks for taking the time to record these! The transparency is appreciated.

FredericHeem commented 8 years ago

What are the technical reasons to remove createClass? IMHO, this factory function is much better than using the class keyword. Binding this in the class is just annoying and require extra work, no need to bind anything with createClass. Second, calling the base constructor is again extra work, in many cases, dev will forgot it, I even come across tutorial that forgot to call the base constructor. Using the class keyword is harder to use and easy to misuse. Please reconsider deprecating createClass, it is just a function, a very useful one.

gaearon commented 8 years ago

What are the technical reasons to remove createClass? IMHO, this factory function is much better than using the class keyword.

I believe the reasons are listed in the notes:

  • createClass() API feels outdated, and can be implemented entirely in userland if needed.
  • ES6 classes are less of an API, possibly simpler.
  • ES6 classes are friendlier to static analysis.
  • ES6 classes offer performance improvements in large apps.
  • We are in the business of UI components, not type systems.

As for

Binding this in the class is just annoying and require extra work, no need to bind anything with createClass.

We are aware of this annoyance. It is also addressed in the notes:

What’s Holding Us Back?

Autobinding
  • Class properties solve this.
  • However they’re not moving forward at TC39 yet as Jeff is busy preparing for React Europe.

Eventually class properties are likely to become a solution to this, but until they are part of the standard, they are indeed “holding us back” from deprecating createClass.

Second, calling the base constructor is again extra work, in many cases, dev will forgot it, I even come across tutorial that forgot to call the base constructor.

Specifying a constructor with ES6 classes in React is not required. You can completely skip it. If you do define it, you won’t be able to “forget” to call super() though because ES6 requires it, and Babel won’t compile the code if you don’t. So I don’t see why this would be an issue.

Using the class keyword is harder to use and easy to misuse.

Can you elaborate on this?

Please reconsider deprecating createClass, it is just a function, a very useful one.

If you like createClass, you will be welcome to use a third-party package that implements it. Per the notes:

  • createClass() API feels outdated, and can be implemented entirely in userland if needed.
  • Eventually we should deprecate createClass, possibly move it into a separate package.

So you’ll still be able to use it if you prefer it to the standard language features.

gaearon commented 8 years ago

Note that in most cases you’ll want to use functional stateless components anyway; not createClass or ES6 classes. Classes are only useful when you want state or lifecycle hooks, which is not every component.

ruiaraujo commented 8 years ago

@gaearon I guess an interim solution would be extracting that functionality to an addons package first so ES6 people don't have to carry those bytes. ;) @FredericHeem using another package would be an acceptable solution, right?

FredericHeem commented 8 years ago

"createClass feels outdated" is not a technical reason but a preference. es6 class are no way simpler because of the required this binding If es6 are more performant, is there any benchmark to assert this claim? Harder to use: we have to mess around the this binding. Easier to misuse: one can easily forgot to call super. There are already tons of code using createClass(), what's the point of breaking source code compatibility? It will create a lot of hassle for absolutely no benefit whatsoever. Imagine of the wasted time and money to convert all the existing code. The code footprint for createClass is probably very small compared to the overall react code so why remove it?

gaearon commented 8 years ago

es6 class are no way simpler because of the required this binding

Conceptually and in terms of the API surface they are simpler. They don't have a special magic API like mixins, merging of their lifecycle methods, statics that get copied onto the class, autobinding etc. I understand some of these are conveniences you enjoy, but this doesn't make createClass simpler.

As for the binding, it is no different from any other JavaScript code. If createClass didn't exist in the first place, and classes were already available when React was created, nobody would call lack of autobinding surprising. It's easy to get used to, but it's not common in JavaScript at all, and lack of autobinding for the purposes of this discussion can be considered simpler.

Again, we understand this is inconvenient. As I just repeated above, these things are still "holding us back". And we hope that class properties will alleviate this pain when they become part of the standard, but this hasn't happened yet.

If es6 are more performant, is there any benchmark to assert this claim?

I can't personally give you any exact benchmarks but I know this has been true for Facebook and Yahoo. This isn't really unexpected: ES classes and createClass follow the same code path after initialisation but createClass() spends time binding all custom methods (as opposed to an opt-in smaller list) and creating class definitions from the spec. It has to be slower by definition because it exists to do more work.

Easier to misuse: one can easily forgot to call super.

I believe I already replied to this in the comment above. You can't forget a super call, it won't compile.

There are already tons of code using createClass(), what's the point of breaking source code compatibility

We can imagine it very well because, as mentioned in the notes, we also have a ton of React components in Facebook using createClass. I would imagine thousands of them.

If we proceed with deprecating it, we will do so in an automated way, with no manual effort, and provide a utility to convert them automatically. In fact such utility already exists: https://github.com/reactjs/react-codemod/blob/master/README.md#class

We will also provide a package for createClass() users who'd rather keep using it. You'll be able to codemod your codebase to use this package instead automatically. Then you won't think about it again.

The reason we want to do it is because we want to reduce confusion caused by having many ways of defining components. createClass() is not something we recommend anymore, so we want to de-emphasise it and remove it from the core. It doesn't mean you'll have to rewrite your code: rather, we will stop recommending it, and it will become like a third party package.

wallclockbuilder commented 8 years ago

ES6 classes are more performant. By how much? 1%? 500%? Are there any benchmarks to back this claim?

gaearon commented 8 years ago

Sorry, I don't have any specific benchmarks to point you to. I think I said it right above:

I can't personally give you any exact benchmarks

Maybe @spicyj or @mridgway could share numbers if they have any.

keyz commented 8 years ago

@wallclockbuilder Here's a benchmark (createClass vs. Component) starting at 12:18 that you might be interested in: https://youtu.be/PnpfGy7q96U?t=12m18s

koba04 commented 8 years ago

Note that in most cases you’ll want to use functional stateless components anyway; not createClass or ES6 classes.

I'm using SFC for most Components, which is very convenient! Unfortunately, I need to use ES6 classes for shouldComponentUpdate where I need some perf optimisations.

Are there any plans supporting shouldComponentUpdate or a flag like pureRender for SFC? I think it would be nice if SFC has the feature.

dzhiriki commented 8 years ago

@koba04 recompose contains pure HOC, which works with SFC.

gaearon commented 8 years ago

Are there any plans supporting shouldComponentUpdate or a flag like pureRender for SFC? I think it would be nice if SFC has the feature.

Eventually, yes. For now, you can use the workaround from https://github.com/reactjs/core-notes/pull/15#issuecomment-220879313.

DanielMSchmidt commented 8 years ago

To the discussion about createClass: To my mind may open source projects use the ES6 Syntax and on Stackoverflow there are multiple cases, where users didn't understand why the docs had the other syntax and assumed there would be a functional difference.

Moving the docs towards ES6 syntax could help here IMHO and I would love to help! Please ping me if help is wished there ;)