Mastodon: React: Update to remove deprecated syntax.

[zacanger]

This was kind of a difficult one to search for so if there's already been an issue around this, please point me to it and close this one!

It looks like all or most of the frontend currently uses createClass, which is now deprecated. Are there any thoughts on moving to pure components and ES6 classes? There are codemods available to help with this, and anecdotes say "they work!" (though I haven't tested those specific codemods myself).

This change should also bring performance improvements.

If this is a thing that seems valuable, I can take a look at it, but it might be a little bit until I have time (sometime in May or June, I'd think).

---

• [x] I searched or browsed the repo’s other issues to ensure this is not a duplicate.

[expenses]

How much work does migrating a React class to an ES6 class require? We've got 73 instances of React.createClass in the code, so this might take a while to do.

[jpdevries]

Would it make sense for updating to ES6 classes to be done in conjunction with moving to Webpack? https://github.com/tootsuite/mastodon/issues/1712

[hendotcat]

I had a bit of a look into this yesterday, with reactjs/react-codemod. I put this in ./bin/jscodeshift. It's the exact options from the codemod ES2015 transform docs applied to every file containing React.createClass.

#!/usr/bin/env ruby

files = `git grep React.createClass`.split(/\n/).map do |line|
  line.match(/^[^:]+/)
end

options = [
  '-t ../react-codemod/transforms/class.js',
  '--flow=true',
  '--pure-component=true',
  '--remove-runtime-proptypes=false',
  '--explicit-require=false',
].concat(files)

command = 'jscodeshift ' + "\\\n\t" + options.join(" \\\n\t")

exec command

It _sort of_ worked, but not well enough to turn into a quick PR. Mainly, the existing browserify setup isn't modern enough and chokes on the static propTypes = {} blocks that the codemod replaces the propTypes declarations with. I looked into upgrading it, but it was a bit of a struggle, and I was worried it wasn't worth the effort given the plans for #1712. Tried to tweak the codemod too, but wow, that was just too hard for me.

Also, the codemod did some very wrong-looking things, which kind of put me off using it at all.

-  handleFollow () {
+  handleFollow = () => {
     this.props.onFollow(this.props.account);
-  },
+  };

-  handleResize () {
-    this.setState({ width: window.innerWidth });
-  },
+  () {
+      this.setState({ width: window.innerWidth });
+    },

I'd be up for doing this work manually, and just doing any static stuff the old-fashioned way like Component.propTypes = {}; after the ES6 class definition. That way it wouldn't require any build tool changes. I can cope with a bit of time consuming manual editing for these 73 files. But it's not something I'd wanna do before getting some kind of green light. What do you think? I'll do it if people want it! 😄

[zacanger]

Ahh what happened to handleResize?! That change to handleFollow would actually work fine with babel-plugin-transform-class-properties but I don't think it _should've_ happened.

[hendotcat]

Ahhh okay, so the mangled handleResize is caused by the @debounce(500) annotation above it. Quite trivial to temporarily comment all of those out before running the codemod, so I'd considered that part fixed ✅

Turns out the other thing, where it converts methods into those strange-looking class property initializers, is by design. Seems to be intended to avoid having to .bind() click handler callbacks and so on. It only does it for methods that _aren't_ React component lifecycle callbacks.

Dunno, personally I find it surprising and intrusive. I figured out how to disable it just now, and after I did, the diff off the codemod's changes felt a lot simpler and cleaner. Like, literally just in and out, swapping the createClass calls with regular classes and nothing else.

Note to self in case I lose this: here's how I disabled that aspect of reactjs/react-codemod.

--- a/transforms/class.js
+++ b/transforms/class.js
@@ -942,11 +942,9 @@ module.exports = (file, api, options) => {
         return createClassPropertyWithType(prop);
       } else if (isPrimProperty(prop)) {
         return createClassProperty(prop);
-      } else if (AUTOBIND_IGNORE_KEYS.hasOwnProperty(prop.key.name)) {
-        return createMethodDefinition(prop);
       }

-      return createArrowProperty(prop);
+      return createMethodDefinition(prop);
     });

The only thing left to figure out now is the JS build config stuff 😮

[hendotcat]

Okay wow! I have a working branch!! 🌈🚀🥂

Take a look at 608dec0. It builds and runs fine locally for me. And it passes the (admittedly quite meagre) unit test suite! And no more annoying console warning about React.createClass deprecation!

In the end I had to re-enable that codemod thing that I'd hoped to turn off. Mainly for stuff like all the setRef methods which are written slightly differently in masto compared to what I'm used to. They depend on the method running in the correct this scope, so we _do_ need that aspect of the transformation.

Gonna do a whole bunch of exploratory manual testing to check if this has broken anything subtle, and then maybe send a PR. Amazing! 😮

[zacanger]

Going to close this since #1905 was merged.