-
Notifications
You must be signed in to change notification settings - Fork 50
Attempt to support native ES2015 classes #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
.babelrc
Outdated
"env": { | ||
"development": { | ||
"plugins": ["transform-decorators-legacy"], | ||
"presets": ["es2015", "stage-0", "react"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than use presets here let's just list plugins explicitly in every configuration? We can then move the shrared plugins up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do. Should I modify package.json as well to include only the required plugins or keep the presets?
Strangely the Travis build fails with |
Is it configurable in modern browsers? Those are the ones we care most about. |
Yes in Chrome, no in Safari. I guess revert then? |
Yeah :-( |
If I keep the switch statement the test passes. This may be OK as a react component can only have at most three constructor parameters AFAIK - props, context and updater. |
I understand but it is still weird that we keep this code to appease a test. |
I tried to debug it but I can't get the source maps to work. Can't find a way to fix the test either. |
Can we maybe define fake |
Inheriting from Function and throwing in the constructor seems to do the trick. |
I just hit this as well. Is anyone still actively working on this? |
This pull request should do the trick. Tests pass. |
Attempt to support native ES2015 classes
I’ll need to backport this to 1.x because that’s what everybody uses right now, hopefully that will be straightforward :-) |
@gaearon code looks quite similar. I can do the backporting. Thoughts? |
I pushed some work in progress in Also, I can’t get Webpack build to work ( |
I just ran |
Nvm. I guess you meant things broke after merging my pull request. Will look into it. |
Apologies, seems like I broke it. I didn’t like that we referenced |
I reverted my commit but I’d appreciate if you could rework it in a way that would be correct later. |
1.x almost works, has one test failing. Mind looking into it? |
I used 'development' because this is the default 'environment' babel runs with. I decided to make the 'harmony' the odd one to avoid specifying the node environment in the npm scripts. We should continue building with babel classes though in order to support older runtimes. I will check the 1.x branch. |
The other solution I can think of is to create two babel environments and duplicate most of the plugins. |
Fixed 1.x. Let me know if you want me to make any changes in the .babelrc. Right now I see the following options:
|
I’d vote for a specific env + duplicating plugins explicitly. |
Out in 1.1.3 (2.0.2 too although nobody uses 2.x yet probably). |
Should fix #41