Only wrap descendants with render method, closes #84

[nfcampos]

closes #84 (see the issue for a discussion of the changes)

todo

• fix current tests
• make change to index.js
• write additional tests

[nfcampos]

@gaearon should i write a new test for each of the cases the current tests used to cover?

[gaearon]

Please do.

[nfcampos]

something like this is not supposed to be wrapped, is it?

import React, { Component } from 'react'
class Foo extends Component {
  render() {}
}

[gaearon]

This looks like a React component to me, why not?

[nfcampos]

because it requires changing this line https://github.com/gaearon/babel-plugin-react-transform/blob/master/src/index.js#L174
to superClasses: options.superClasses || ['React.Component', 'Component'],
which mean that any class that extends a class called Component and has a render method is wrapped.
is that ok? or is there some other way of doing this?

[gaearon]

Good catch. I would say let’s add Component to defaults. It’s a too common pattern.

[nfcampos]

yeah i agree. does that go into this PR or another one (it's not really related to this)?

[gaearon]

It’s fine to do this here.

[nfcampos]

Alright, it's all done i think.
changes:

• only wrap classes that extend the correct superClass and have a render method
• changed tests to reflect change above
• added Component to default superClasses
• added test for the above

[gaearon]

Seems like technically this wouldn’t even be a breaking change because we used to allow any class with render() anyway.

[nfcampos]

well it breaks some stuff that shouldn't work anyways, so that part is fine

to me it looks like the only breaking change would be stuff like this:

import React from 'react'
const Comp = React.Component
class Foo extends Comp {
  render() {}
}

this would have worked previously because it had a render method and now won't, but would anyone actually do something like this?

[gaearon]

Even if they used this pattern, we’re not technically “breaking” it because the code would still work. Transforms are meant for development only anyway.

[nfcampos]

yeah that's true

[gaearon]

Out in 2.0.1, thank you a lot!

[nfcampos]

Happy to help!

[gaearon]

If you have time, I’d appreciate you looking into why #87 fails, merging/updating it if it’s correct, and checking whether it fixes another correctness issue: #88.

[nfcampos]

I'll look into it