Call clearInterval after setInterval when needed

[mvandenburgh]

This fixes a bug where the setInterval would continue firing after the user navigates from one dandiset to a new dandiset. This would result in unexpected behavior, such as the overwriting of the new dandiset with the metadata from the old one retrieved by the setInterval call.

One note about the changes - I changed the use of setInterval to window.setInterval. I couldn't get TypeScript to play nicely with the return value of setInterval for some reason; it was trying to get me to import a type from NodeJS, but since we're in a browser environment it was erroring out. They are functionally identical though.

[mvandenburgh]

Vue docs explicitly mention onUnmounted as the correct lifecycle hook for this btw https://vuejs.org/api/composition-api-lifecycle.html#onunmounted

Use this hook to clean up manually created side effects such as timers, DOM event listeners or server connections.

[waxlamp]

When you changed the function from setInterval() to window.setInterval(), did the typechecking errors go away? This Stack Overflow question (which you may already have seen) talks about the difference between the web and Node versions of setInterval() and suggests a TypeScript idiom that would work for both environments (and allow you not to include the window. prefix).

[mvandenburgh]

When you changed the function from setInterval() to window.setInterval(), did the typechecking errors go away? This Stack Overflow question (which you may already have seen) talks about the difference between the web and Node versions of setInterval() and suggests a TypeScript idiom that would work for both environments (and allow you not to include the window. prefix).

Yeah, I saw that SO answer. I couldn't get it to work, it causes this error for me -
Property '__promisify__' is missing in type 'Timer' but required in type 'typeof setInterval'.ts(2741)

[waxlamp]

If others are happy with this, then it LGTM.

[waxlamp]

To the extent that we think it's actually an error condition if the timer is not set at this point, this is fine. If it ever happens that we want to cancel the interval timer for some other reason, then I think we would invert this conditional, remove the throw statement, and not worry about a non-existing timer.

UPDATED TO ADD: I understand this code appears this way largely for typechecking reasons. My suggestions about the importance of recognizing this as an error condition would preserve the typechecking functions while omitting the raising of an error.

[mvandenburgh]

I'll also add, the throw statement has the advantage of causing a sentry error if it somehow ever did get called.

[dandibot]

🚀 PR was released in v0.3.33 🚀