Update to React 16 and move to ES6 #26
Conversation
sebacruz
changed the title
|
Can you merge this @kimmobrunfeldt? |
|
I would really appreciate mergin this PR |
|
Me too! |
|
I tried to be contributor of the repository for fix theses necessary PR's but @kimmobrunfeldt is not replying 😢 |
|
@Kikobeats - Bah, that sucks!! |
|
@erosenberg I updated the |
|
@sebacruz - looks good now, thanks!! |
|
What do you think create a fork package of this? something like: https://www.npmjs.com/package/react-bar-progress This situation is so tidious |
|
I've forked this repo here: https://github.com/squarecat/react-progressbar.js and it's now working in my React 16 project. I've applied the patch of this PR, and given the repo public push access, so if anyone wants to move over any of the other PRs then feel free. It's published to npm under |
|
Thanks for your effort @Jivings! I'm seeing another error related with this line. My code said: I think this is the correct way to setup a ref node: It's used by the Shape internal components: |
|
@Kikobeats Yep you're right. I also had to follow the answer in this thread. I think this is due to The alias obviously forces both the imports to use the same React version. |
|
@Jivings hmmm I'm not sure. Probably the code should be write as ES6 using imports and be shipped using babel compiler to ensure module injection is working as expected. I wrote a package using this pattern, you can check the code here: https://github.com/Kikobeats/react-img-atv I'm going to try create a PR doing that |
|
Yep I agree. These are just workarounds for a component that hasn't been built to be particularly resilient to it's environment. |
|
Would love to use this library, but my project's React 16. Looking forward to this some day getting merged? |
|
@mssngr uses the @Jivings forks, it is working like a charm Could be great release it as Feel free to use https://www.npmjs.com/package/react-bar-progress as well 🙂 |
|
@mssngr @Kikobeats in the meantime you guys can just do: Would be nice for a proper merge though as this is fragile from @squarecat |
|
@jmfurlott I actually published the fork to npm so you can do; @mssngr Unfortunately it looks like this project has been abandoned, but if anyone wants to contribute to the fork then let me know and I'll just grant write access. |
|
I'm sorry that I haven't been able to keep up with the maintenance. If @Jivings or others would be interested to help with this repo, I'm totally open to give access. |
| var style = this.props.containerStyle; | ||
| var className = this.props.containerClassName; | ||
| render() { | ||
| const {style, className} = this.props; |
There was a problem hiding this comment.
@sebacruz should be containerStyle and containerClassName
const { containerStyle, containerClassName } = this.props;
Both updates are related somehow so I put it in the same PR.
Also this should fix #24.