Minimum changes to support React 16 #78
Conversation
|
Paul Armstrong seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
paularmstrong
force-pushed
the
parmstrong/fork-react-16
branch
from
5e9b7d1 to
e5d9ac8
Compare
|
@paularmstrong aren’t the tests 16.x only? (different enzyme adapter) |
|
@diligiant Yes, but I don't see the necessity of creating separate enzyme adapters for 15.6 and 16.x and running the test suite twice, unless there's some sort of regression found. We're using this on Twitter Lite successfully on 15.6 right now. |
|
Then shouldn’t this be R16.x only ? How can “it” claim to be compatible with 15.6 if it is not able to test itself with this version ? you and I know it works because we’ve done the manual changes to be sure but that’s irrelevant to people out there who can’t reproduce easily what we’ve done. On another topic, what do you have in mind to move forward ? Another PR for Rollup and another one for what’s left ? (I’m new here so I ask ;) |
.gitignore
Outdated
| @@ -1,2 +1,3 @@ | |||
| node_modules/ | |||
| dist/ | |||
| .DS_Store | |||
There was a problem hiding this comment.
Remove? People should have this globally ignored
| displayName: Fn, | ||
| componentWillMount: function() { | ||
| return class extends React.Component { | ||
| // static displayName = Fn; |
There was a problem hiding this comment.
This is the Outstanding issues from the description. Esperanto chokes when this is hit, even with the babel plugin.
There was a problem hiding this comment.
Does that mean we need the tool improvements too?
There was a problem hiding this comment.
They will be needed if you want to support displayName. But that's really only for debugging purposes, so it's not a blocker for things to work.
|
Waiting for @paularmstrong's response on #78 (comment) |
paularmstrong
force-pushed
the
parmstrong/fork-react-16
branch
from
e5d9ac8 to
9b7b950
Compare
How should we test 15.x? I believe this is related to #77 |
|
Apparently grunt/esperanto doesn't actually run babel across the files? That explains the The build system needs to be overhauled first. ¯_(ツ)_/¯ |
Problem: #76 makes too many changes, does much more than just supports React 16.
Solution: Apply only the minimum changeset to allow react-globalize to work with React 15.6.x and 16.x.x
Outstanding issues: The build system looks like it will actually need to be changed, but separately, because esperanto chokes on babel-plugin-transform-class-properties.