-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
Material-UI not compatible with React 0.14 #1030
Comments
@tleunen You're on the ball! React 0.14 beta was released today. @hai-cea I took a look at some of the issues (I found ~50 across 21 files). This could be a good opportunity for a milestone. I was looking at the dialog/dialog-window/overlay files and there's a lot of coupling via this.refs.xxx.yyy() so this may not be a simple task. If you want to make a React 0.14 Compatibility milestone let me know and I can assist in writing issues (I'm sure @tleunen would like to be involved -- I don't want to volunteer him). |
Thanks guys @tleunen! I agree, this will cause some problems. What's the alternative? Save the element into a variable inside render? @jkruder Are you on gitter - https://gitter.im/callemall/material-ui ? Please send me an IM on there and we'll get the ball rolling on organizing issues/milestones. |
Most of the time, components should not have public functions. Everything has to be passed with props or when they are mount. For example, the Dialog component doesn't need I'm just starting using Material-UI so I'm not much aware of issues with others components, but I think most of them could be rewritten to not have public functions (Anyway there is no other choices). |
@tleunen I can't agree more on the Dialog show and hide methods. |
@tleunen @oliviertassinari Yeh, I agree with you both. This was a debate we had when designing that component. The issue that we had was the clickaway feature. If open/close was handled in state, then it could worry about closing itself on clickaway. If open/close were passed in as props, everyone using dialog would have to handle clickaway themselves. Now a middle solution would be to add an onClickAway prop to dialog and let the user of the component open/close. |
@hai-cea I would suggest to use the same approche as https://github.com/rackt/react-modal. This is basicaly your middle solution. |
What other thing does this library use |
You mean that if React detect the ref is on a custom component, it will return the component instead of the DOM element inside the component? |
@tleunen that is my understanding. Needs verification :) |
@mull That would be very nice if that's the case. :) |
@hai-cea @mull @tleunen Just ran a quick test and ref on a custom (something that extends React.Component) component (this.refs.customComponent) will return a reference to the React component NOT the underlying DOM node. If you have a ref to a DOM node (div/a/img/etc) then this.refs.domRef will return the node. @hai-cea That being said, I still think it's a good idea to move away from calling methods on this.refs.XXX. |
@jkruder thanks, glad I wasn't talking out of my ... :) |
I guess we can close it then. But it should be good to rewrite some of the components to remove the need of calling functions on them. It's not how components should work :/ |
@hai-cea Agreed. I'm working on a draft of the proposed work for 0.14 milestone that I'll send to you for feedback. In regards to #1033, I don't think we should make the jump just yet. I'm all for creating a separate branch where we can convert MUI so that it's compatible with what is proposed for 0.14 and make the components more functional (minimize/eliminate this.refs.XXX.YYY()). |
If the current use of this.ref.xxx doesn't actually break material-ui when used together with react 0.14.0-beta1 then I don't feel the wish to move away from that pattern should block #1033. By making it easier to install material-ui alongside react 0.14.0-beta1 you'll set yourself up to receive early feedback on actual issues that may arise; it's better to receive that feedback when 0.14 is still in beta. Perhaps a good alternative (that better manages expectations), is to release an alpha/beta/rc version of material-ui on npm that has 0.14.0 as a peer dependency (and is geared towards making material-ui 0.14 compatible). That way it's easier for people to move forward and find/fix any issues that might exist. |
@jkruder Any update on this? |
@ashtonwar None yet -- I've been focusing on getting some testing established and resolving some external commitments. I should have some time this week to take a look at it and see what we're working with. Some others have already made some attempts at migrating to 0.14. |
Cheers for looking at this. I've seen |
No problem; I still believe that it is best to avoid using |
Any updates? React JS 0.14 RC 1 just got released and really would like to use Material-UI with it. |
same here, is there a way we can support the migration to 14? |
I'm using material-ui-io in production - it seems OK. |
+1. Would also like to know the timeline of this update! |
👍 |
We're getting there, folks! See #1751. At this point a little more work is needed to upgrade to the new I would recommend trying out the |
Great to hear! Look forward to a final version. I have been working with On Tue, Sep 29, 2015 at 1:31 PM, Shaurya Arora notifications@github.com
|
Cool! I will be putting issues if there is anything... |
Any news on a release date for React 0.14 support? |
@amagdas are you aware of the |
@shaurya947 Yup, I'm aware of it, but I'm not able to install the branch using npm, trying again. |
you can either |
To be clear you need to do the npm install in the node_modules directory
|
@amagdas this branch is not up on npm yet as it still has some issues and is a work in progress. You can either do what @clkao said, or, after cloning the repository on your machine, switch to the After that when you run |
Yup, done this and it works, will give feedback. |
How about leveraging this FB tool to automatically make the changes? https://github.com/facebook/react/blob/master/packages/react-codemod/README.md |
Look at "Notable bug fixes" in react 0.14 announce (http://facebook.github.io/react/blog/2015/10/07/react-v0.14.html): |
@kinolaev For more detail: #1732 (comment) |
Is |
@ovaris I was merged to master. |
@oliviertassinari when will be available on npm? |
I have no idea when we will have a bug free version. However you can try master branch with npm. |
Should be fixed with the latest release v0.13.0 |
@oliviertassinari thanks! |
merci! |
Any Ideas if this issue will still exist in the latest version of material-ui ? I can use most of the components but the ones which use this.refs.xxx. e.g if I try to use DatePicker Component, I get an error "Cannot read property 'show' of undefined" and undefined here is this.refs.dialogWindow. I am on react 0.14.8 and material-ui 0.14.4 ... |
the same error |
Seems like material-ui does not work with React 0.14.8 and 0.14.9 and that is pathetic. |
@topgun743 That's very hurtful of you to describe the excellent work here (given away for free) as pathetic. Since this issue was opened React 15 was released, which material-ui is compatible with. I suggest upgrading React inside your project. |
Because of the change on the
refs
that React 0.14 has. I think Material-UI will have quite a lot of issues with almost every component of the library.the
this.refs.XXX
will return the DOM node instead of the component, which means we won't be able to call any component functions on it.The text was updated successfully, but these errors were encountered: