NdYAG/react-rater

Bug with complex child elements

nrempel opened this issue · 11 comments

I'm using this library with complex child components and it throws an error when I click on my element.

index.js?33a0:106 Uncaught TypeError: Cannot read property 'indexOf' of null

I traced the error to the following line:

https://github.com/NdYAG/react-rater/blob/master/src/index.js#L58

A quick and dirty fix is to replace

if (rating < 0 || e.target.getAttribute('class').indexOf('is-disabled') > -1) {

with:

if (rating < 0 || e.target.getAttribute('class') && e.target.getAttribute('class').indexOf('is-disabled') > -1) {

I don't have time right now to submit a failing test for this case and a fix - but I wanted to let you know. Thanks for the great library!

NdYAG commented

@nrempel Thank you! I'll work on this soon.

NdYAG commented

Hi @nrempel
b62d02f#diff-1fdf421c05c1140f6d71444ea2b27638R58
I've update the package on npm to 0.3.1,
try update to see if it works for you.

Great! I'll give it a try.

NdYAG commented

@nrempel Shall I close the issue?

I now get the following error:

Uncaught (in promise) Error: addComponentAsRefTo(...): Only a ReactOwner can have refs. You might be adding a ref to a component that was not created inside a component's `render` method, or you have multiple copies of React loaded (details: https://fb.me/react-refs-must-have-owner).
    at invariant (eval at <anonymous> (http://localhost:5000/bundle.js:642:2), <anonymous>:38:15)
    at Object.ReactOwner.addComponentAsRefTo (eval at <anonymous> (http://localhost:5000/bundle.js:6954:2), <anonymous>:67:79)
    at attachRef (eval at <anonymous> (http://localhost:5000/bundle.js:6966:2), <anonymous>:23:16)
    at Object.ReactRef.attachRefs (eval at <anonymous> (http://localhost:5000/bundle.js:6966:2), <anonymous>:42:5)
    at attachRefs (eval at <anonymous> (http://localhost:5000/bundle.js:1230:2), <anonymous>:22:12)
    at CallbackQueue._assign.notifyAll (eval at <anonymous> (http://localhost:5000/bundle.js:3456:2), <anonymous>:66:22)
    at ReactReconcileTransaction.ON_DOM_READY_QUEUEING.close (eval at <anonymous> (http://localhost:5000/bundle.js:6960:2), <anonymous>:79:26)
    at ReactReconcileTransaction.Mixin.closeAll (eval at <anonymous> (http://localhost:5000/bundle.js:2064:2), <anonymous>:202:25)
    at ReactReconcileTransaction.Mixin.perform (eval at <anonymous> (http://localhost:5000/bundle.js:2064:2), <anonymous>:149:16)
    at ReactUpdatesFlushTransaction.Mixin.perform (eval at <anonymous> (http://localhost:5000/bundle.js:2064:2), <anonymous>:136:20)

The error references this document: https://fb.me/react-refs-must-have-owner

It might be related to https://github.com/gaearon/redux-thunk based on what I see in the stack trace.

NdYAG commented

I could not see the reason why assigning ref dynamically in render would throw this error.
It works in the customize example in example/index.html.

What does the child elements in your code look like?
Or maybe would you try to reproduce the error without redux-thunk? Then I'll try to debug it.

NdYAG commented

Hi @nrempel
Based on http://stackoverflow.com/questions/28519287/what-does-only-a-reactowner-can-have-refs-mean/32444088#32444088, maybe the error message throws out due to two react dependencies.

If you are using redux, how about trying the author's solution?

Using e.target.getAttribute('class').indexOf('is-disabled') may not be a good solution, as we're making the parent element guess the DOM of customized child elements.

NdYAG commented

I'm closing this issue as it seems that the first bug has been resolved. Thanks again the bug reporting @nrempel 😉
Reopen it any time if you encounter any other error message.

Yes, thank you very much for fixing that @NdYAG!

Right now I'm using my quick fix since I don't see any errors.

When I have more time, I will come back to the other error I see and try to propose a solution.

NdYAG commented

Hi @nrempel

I shall feel guilty for closing this issue early arbitrarily.

Truly it's due to multiple version of React in the bundled script. But it's my fault adding react and react-dom to dependencies in package.json. Now I've moved them to peerDependencies and update version to 0.3.2 on npm.

If you have time, please update to the latest version. I believe you will not meet the error again.

Ok, thanks @NdYAG, I'll try the new version when I have time.

Thanks for looking into it!