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

Avoid using Promise.prototype.finally to better support older devices #1224

Merged
merged 2 commits into from
Feb 27, 2023

Conversation

peaBerberian
Copy link
Collaborator

Starting with the for-now-unreleased v3.30.0 release, due to the fact that we completely removed RxJS from the code, we started relying much more on Promises and on its related methods.

After encountering a curious issue on Panasonic TVs while investigating #1219, we saw that the finally method on Promises was poorly supported on some devices, and to our surprise even if a Promise polyfill was added.

After evaluating what we could do, I decided to just ban the finally method from the RxPlayer code and replace it by duplicating its logic in Promises' resolve and failure handlers.
To ensure that those methods are not used on future developments, I also added an eslint rule (and plugin) to ban the finally method. Sadly, this plugin does not check if a finally method call is actually done on a Promise instance, so I had here to just ban the method altogether (to prevent future confusion, I clearly indicated in the error message that the linting rule was naively banning all finally method calls).

I also tried to override a Promise's finally method through TypeScript, but this solution appeared much more hacky than I first thought. I think a linting rule is a better solution for now.

@peaBerberian peaBerberian added this to the 3.30.0 milestone Feb 24, 2023
@peaBerberian peaBerberian force-pushed the ban/promises-finally branch 2 times, most recently from f7a098e to 4a971ac Compare February 24, 2023 14:50
@peaBerberian peaBerberian merged commit e67932e into next Feb 27, 2023
@peaBerberian peaBerberian deleted the ban/promises-finally branch July 6, 2023 12:03
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

Successfully merging this pull request may close these issues.

None yet

1 participant