Reporting initial work #3712
Reporting initial work #3712
Conversation
| // set timeout for raising alert to refresh the token 2 minutes before the session expire | ||
| timer = setTimeout(() => { | ||
| console.log('Calling refresh looker session action') | ||
| thisRef.props.refreshLookerSession() |
There was a problem hiding this comment.
hint: this inside arrow functions () => {} always point to the outer this, so we don't have to use thisRef and can use this instead, it would work.
@vikasrohit fyi
There was a problem hiding this comment.
I always found the this in side the native javascript methods (like setTimeout), ambiguous. But as it is working for this case as you suggested and I double checked, I am removing the need of this Ref.
| return !props.isLoading | ||
| })(LookerEmbedReport) | ||
|
|
||
| let timer |
There was a problem hiding this comment.
Generally, it's safer to keep timer inside class like this.timer. Otherwise, if we have 2 components on one page, they would use the same timer and break each other.
fyi @vikasrohit
There was a problem hiding this comment.
Agreed. I placed it intentionally outside because I do want to use the same time for all reporting components on the same page.
However, on second thought, I think we can move it to inside the class for better reading because I don't expect multiple copies of this component on the same page.
|
Implemented both suggestions. Thanks @maxceem |
fyi @maxceem