Skip to content
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

Document nullish timer argument for cancel() #795

Open
andreyfel opened this issue Feb 24, 2022 · 1 comment
Open

Document nullish timer argument for cancel() #795

andreyfel opened this issue Feb 24, 2022 · 1 comment

Comments

@andreyfel
Copy link

Current documentation doesn't mention that the timer argument can be undefined/null:
https://api.emberjs.com/ember/4.1/functions/@ember%2Frunloop/cancel

However, the implementation allows that:
https://github.com/BackburnerJS/backburner.js/blob/af77b18f37ec24e89f65bef0ce16bc3c8d1f4bfb/lib/index.ts#L551

In fact, in our code, we were leveraging the fact that you can pass undefined as the timer argument. It may be useful if you have a local var where you save the timer, so, the type f this var is EmberRunTimer | undefined because the timer is not initialized in the constructor. In willDestroy hook we call cancel(this.timer).

In fact, we should understand if we can keep doing that or we cave to wrap such calls with if (this.timer) check.

@ember/runloop types do not allow undefined, this is where this issue came from. DefinitelyTyped/DefinitelyTyped#58893

@chancancode
Copy link

Personally, I think the app code should write the conditional for clarity, but I think the intention here is to match the DOM apis for clearTimeout etc, and since TS also marks that as ? it seems fine to do the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants