Comments (10)
I think the problem is here.
Most likely that these two lines should end with ||
rather than &&
:
babel-plugin-react-transform/src/index.js
Line 77 in 63cafc9
babel-plugin-react-transform/src/index.js
Line 122 in 63cafc9
Please feel free to submit a fix, add tests for it, and I’ll be happy to merge.
from babel-plugin-react-transform.
Why is the expected behaviour in this test wrapping the class? Not all classes with a render method are React components
https://github.com/gaearon/babel-plugin-react-transform/blob/master/test/fixtures/code-class-with-render-method/actual.js
To me it looks like that's one of the tests I wanted to add (expecting it not to wrap the class) to make sure #81 and #78 were fixed, no?
from babel-plugin-react-transform.
We should expect that
class Foo {
render() {}
}
does not get wrapped.
Same for
var Foo = PropTypes.shape({
render: PropTypes.func
})
It also should not be wrapped.
from babel-plugin-react-transform.
Exactly, that's why this test can't be right https://github.com/gaearon/babel-plugin-react-transform/blob/master/test/fixtures/code-class-with-render-method/expected.js
Also, none of these should be wrapped either, right?
https://github.com/gaearon/babel-plugin-react-transform/blob/master/test/fixtures/code-call-expression-with-render-method/actual.js
from babel-plugin-react-transform.
Exactly, that's why this test can't be right
Yeah. It’s wrong.
Also, none of these should be wrapped either, right?
They shouldn’t. However they should be wrapped if factory
is specified as one of the values of the optional factoryMethods
array config option.
from babel-plugin-react-transform.
It looks like to me that this line shouldn't even be there
babel-plugin-react-transform/src/index.js
Line 78 in 63cafc9
We never want to wrap a class that doesn't extend React.Component (we might if we wanted to support extending a class that itself extends React.Component but whatever), right? If it doesn't pass that superclass test there's really no reason to check whether it has a render method is there?
from babel-plugin-react-transform.
If you change it to be ||
then I think it makes sense. "If either it's not a descendant -or- it doesn't have a render method, bail out because it is definitely not something we can wrap."
from babel-plugin-react-transform.
the goal:
if (shouldNotWrap)
return
else
wrap()
a) current implementation:
if (doesntExtendSuperClass && doesntHaveRenderMethod)
return
else
wrap()
// !(doesntExtendSuperClass && doesntHaveRenderMethod) === (extendsSuperClass || hasRenderMethod)
b) change to ||
:
if (doesntExtendSuperClass || doesntHaveRenderMethod)
return
else
wrap()
// !(doesntExtendSuperClass || doesntHaveRenderMethod) === (extendsSuperClass && hasRenderMethod)
// so this would only wrap descendants of the super class that have a render method
// which makes all the current tests (but one) fail
// but is maybe what we want actually, seeing as the render() method is required in react classes (see https://facebook.github.io/react/docs/component-specs.html#render
c) remove the render method check:
if (doesntExtendSuperClass)
return
else
wrap()
// !(doesntExtendSuperClass) === (extendsSuperClass)
// so this would wrap all classes that are descendants of the super class
// which fails only 2 of the current tests (the 2 tests that are expecting the wrong thing)
so which one do we want? (b) or (c)?
from babel-plugin-react-transform.
I think we want (b). Only descendants with render methods are good to go.
from babel-plugin-react-transform.
Should be fixed in 2.0.1.
from babel-plugin-react-transform.
Related Issues (20)
- Local path as an argument to "transform" does not resolve the path.
- Modify the contents of the function will not be updated.
- Doesn't work with React.PureComponent HOT 1
- Template is not a function
- Deprecated with no alternative? HOT 1
- no release info or changelog for 3.0.0
- `addImport()` is deprecated in Babel 7 HOT 2
- Use without .babelrc HOT 4
- jspm with plugin-babel and react preset: "cannot read property 'transform' of undefined" HOT 1
- React transforms being run on Backbone views HOT 6
- Question: Is it a bad practice to use react-transform for production code? HOT 1
- Transform(s) to strip propTypes and displayName HOT 4
- Ember objects detected as react components HOT 5
- error when passing properties to createClass() HOT 4
- Future of React Transform HOT 2
- Can we have superClass Regexp matching option , for inheritance ? HOT 2
- undefined value static properties outside class HOT 3
- displayName equals 'Constructor' HOT 8
- static properties are undefined HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from babel-plugin-react-transform.