React-quill: Future React compatibility

[alexkrolick]

Opening this issue to discuss the changes coming in React 16:

• createClass will be moved to a separate module. We'll have to add it is a dependency if we want to keep using the mixin approach.

Related:

• https://facebook.github.io/react/blog/2017/04/07/react-v15.5.0.html

[zenoamaro]

That would be the most practical thing for the moment.

However, thinking of it, none of the contents of the mixin are overlapping with destination class methods, and actually, it does not need to be a mixin at all – just an plain old object that can be Object.assigned.

I suggest we let go of createClass when we switch to classes, but otherwise we could already abandon the mixin approach.

[baptistemanson]

We have some bandwidth to make it happen. The PR has a risk to be massive and be hard to review. Any thoughts about how to proceed?

[alexkrolick]

I think we will release 1.0.0 first, then decide what API changes are acceptable - ideally the rewrite would be semver-compatible (1.1 instead of 2.0), although that might not be possible.

The React peer dependency will also be interesting if we need createClass for >=16 but not below. Might be best if we rewrite in ES6 first, actually.

Thoughts?

[zenoamaro]

createClass

We should be able to keep createClass (and propTypes) for the time being. It shouldn't cause any breakage, as it's got a peer dependency on react@>=0.14.0, which we also have. I see no issues in doing this now.

Mixin

Regarding the mixin, it is just a plain object, and it does not overlap with any of the methods of the component, so it can simply be Object.assigned in the constructor or to the prototype when we switch to ES6 classes.

Mixins are deprecated, anyway, and furthermore, this one is no more than set of utility functions disguised as a mixin. I'd refactor it, along many of the components' own methods, into exported utility functions that are called, and not mixed in, by ReactQuill, and tested independently.

If we do this, it's major-worthy, unless we keep the object around for compatibility – not a huge deal. However, I wonder how many people actually use it. Not a hard task, but no party will be ruined if we wait.

ES6 rewrite

The rewrite to ES6 itself sounds like the most probably cause of breakage. It should be invisible to current users, per-se, but we could mess up the bundle, debuggability, source maps, etc. We will for sure anger somebody, somehow. I think it's best if we take our time on this one.

Timeframe

I think that, if we act wisely, all the above can be patch-worthy at most. If we mess something up, but manage to fix it, per SemVer we can simply deprecate and release another patch.

In short, I think most of these can wait after the v1 release, and be dealt with in no hurry.

Anything I'm missing?

[alexkrolick]

Sounds right to me.

Only thing is the peer dependency is ^0.14.0 || ^15.0, but if createClass is added, >=0.14 would work (it's compatible with 16). We can then reduce bundle size for <16 users by importing createClass from React itself at compile time, if dynamic requires are allowed.

Agree that the mixin functions could be reorganized into an inner component, utility module, or something else, and that will probably be a breaking change.

After that we could try running the React codemods to transform the component to the new ES6 React APIs. The exposed API shouldn't have to change, but we should take the time to be picky about how the component is represented internally.

And yes, this can all wait til after 1.0.

[gugadev]

Are you working on this right now or just is part of the roadmap by now?

[czebe]

+1 for this to make the PropTypes warning go away.

[rooch84]

I'm seeing a core-js warning on my build, and it looks like create-react-class is now depending on an out-of-date fbjs package.

└─┬ [email protected]
  └─┬ [email protected]
    └─┬ [email protected]
      └── [email protected]

Given that create-react-class is no long maintained by the react team, I'm just wondering whether you still need it in this package.

[alexkrolick]

Where did you see it no longer being maintained? https://www.npmjs.com/package/create-react-class has only FB contributors

[alexkrolick]

At some point, rewriting this library with new features would be nice, but right now it works all the way back to React 0.14

[rooch84]

I read it here (2nd post)... https://github.com/facebook/react/issues/11024#issuecomment-333495149

Also, from @zenoamaro's comment above, it sounded like you could do without it and not affect functionality.

[zenoamaro]

ReactQuill v2 is now fully TypeScript and React16+.

Please try the latest v2 beta. The upgrade should be drop-in.