Move a few components to es6 with mixin decorator #2436
Conversation
|
I think that we should wait to release v0.14 before migrating to ES6 class. |
|
That's fair; this was just a swing to see how much the codemod tool could do with or without the mixins, and try and move a few components over. Realistically, I believe the whole library could be moved in a couple of hours of fairly idle work, just from trying this. This also kind of proves that nothing breaks; everything behaves exactly the same as before. |
|
😱 I think this is worth it 👍 👍 but 0.14.0 cannot contain this. we'll have to clean up some stuff and PRs before we merge this. otherwise we'll have a hellstorm of conflicts :D |
|
I'd be more than happy to move some more components over and keep adding to this PR, or I could deal with it however you think is best (I can totally understand not wanting one very bloated PR, so I could split it up or something). |
|
I wrote my comment before you edited yours @subjectix. I completely agree, I don't think a syntax change on the entire library should be put into a release that's in an RC phase. |
|
@BobertForever I have another suggestion if @oliviertassinari agrees. I think it's best to add an es6-classes branch, do all the migration, planning, etc there and when all is ready merge with master. |
Sounds ok to me. @shaurya947? |
|
Cool! I can re-direct this PR there when you make it. |
|
@BobertForever Please redirect this to the es6-classes branch. thanks 👍 @oliviertassinari, @shaurya947 Agreed with this in gitter 😬 |
This PR moves
app-bar,app-canvas, andavatarto es6 style classes with a stage-1 decorator to handle the mixin. Most of the code changes were done with react-codemod with some minor changes to account for things in es7 syntax the tool does not account for (likestaticproperties on the class).Each component took about 1 minute to migrate from es5 to es6/es7.
No behavioral changes were introduced, the demo still runs as expected, and all tests still pass.
This PR is in reference to #458.