llaske / ExerciserReact

React code of Exerciser Activity. GSoC 2018
Apache License 2.0
15 stars 49 forks source link

Multimedia JSX #79

Closed AvinashAgarwal14 closed 5 years ago

AvinashAgarwal14 commented 5 years ago

@thingthing I have added a component for redundant multimedia JSX in this PR.

AvinashAgarwal14 commented 5 years ago

@thingthing I have made all the aforesaid changes, but there is still an issue. There are warning regarding memory leaks. If you open any exercise( let us say one of the default ones - Learn Roman Numerals) then close it, there are memory leaks warning in the console. I am not able to track it back to the cause. Could you please help me out.

thingthing commented 5 years ago

@thingthing I have made all the aforesaid changes, but there is still an issue. There are warning regarding memory leaks. If you open any exercise( let us say one of the default ones - Learn Roman Numerals) then close it, there are memory leaks warning in the console. I am not able to track it back to the cause. Could you please help me out.

I'll try to look at it tomorrow, can you give me the exact error you get so that i can see what the issue is? (just a copy paste of the error should be ok)

AvinashAgarwal14 commented 5 years ago

@thingthing I have made all the aforesaid changes, but there is still an issue. There are warning regarding memory leaks. If you open any exercise( let us say one of the default ones - Learn Roman Numerals) then close it, there are memory leaks warning in the console. I am not able to track it back to the cause. Could you please help me out.

I'll try to look at it tomorrow, can you give me the exact error you get so that i can see what the issue is? (just a copy paste of the error should be ok)

Sure, It's a warning regarding memory leak.

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method.
    in REORDERPlayer (created by Connect(REORDERPlayer))
    in Connect(REORDERPlayer) (created by Route)
    in Route (created by withRouter(Connect(REORDERPlayer)))
    in withRouter(Connect(REORDERPlayer)) (at WithMultimedia.js:156)
    in MultimediaHoc (at Router.js:51)
thingthing commented 5 years ago

Ok, cool, it's what i though (i think). The issue is that here you set an interval in a local variable, then you put it in your state, and you use the state one to disable when the component unmount. What i think is that, since setState is asynchrone, you have a intervalId not set in the state yet, but you're unmounting, so the interval is never cleared. Since you don't use this variable in your render, you don't need to put it in your state, you can just use it as a class attribute (this.intervalId = setInterval.... and in unMount clearInterval(this.intervalId)). It should correct it. If not, you should put some log in your interval function and the unmount function to see what is going on, but this error is generally tied with a setTimeout/setInterval that is not cleaned.

Hope this helps!

thingthing commented 5 years ago

Hmkay, what i said before stand, so you should just use a class attribute instead of the state for this interval, but it seems the error is simpler than that: The key of the state object that is set is called intervalId, but the clearInterval is trying to stop the this.sate.intervalID. As you can see, the clearInterval has ID in uppercase while the state does not...

AvinashAgarwal14 commented 5 years ago

@thingthing Yes, it appears the issue was with clearInterval. Thank you, the problem is now solved. :smiley:

llaske commented 5 years ago

Very great. Thank @thingthing for your help.