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

Replace Excalibur promise implementation with browser implementation #994

Closed
3 tasks
eonarheim opened this issue May 22, 2018 · 4 comments · Fixed by #1707
Closed
3 tasks

Replace Excalibur promise implementation with browser implementation #994

eonarheim opened this issue May 22, 2018 · 4 comments · Fixed by #1707
Assignees
Labels
api change Breaking api change

Comments

@eonarheim
Copy link
Member

eonarheim commented May 22, 2018

Context

Currently we have a basic functioning custom promise implementation, but this prevents new browser features like async/await from working natively. Additionally we will get a bunch of promise features for free by switching to native promises. It may also come with a bonus of closing these #533 #365 #341

Proposal

  • Add the tsconfig native promise polyfill option lib: ES2018.Promise
  • For backwards compatibility for other games, change ex.Promise implementation to wrap the native one.
  • Replace existing methods in excalibur that use ex.Promise and use the native Promise returns (this will be a breaking change, unless use the ex.Promise wrapper, there can be no easy transition unless we rename methods)
@eonarheim eonarheim added the api change Breaking api change label May 22, 2018
@eonarheim eonarheim added this to the 0.18.0 Release milestone May 22, 2018
@LoserAntbear
Copy link
Contributor

LoserAntbear commented May 30, 2018

Looks like we also need to add lib: [es2015, dom]. Because es2018.Promise adds final method to Promise interface, defined in es2015.promise PromiseConstructor, if i understand this right.
BTW, I already made this changes and also replaced all uses of ex.Promise with native es6-promise and ready to push here. But i am a bit confused with providing backwards compatibility to ex.Promise. Now i just added as prop ex.Promise._nativePromise which resolves on ex.Promise.resolve and rejects on ex.Promise.reject, but i feel it is not the way it is meant to be. :)

@LoserAntbear
Copy link
Contributor

Hello again! I am sorry, bear with me. :D Could you look into this, please?

@eonarheim
Copy link
Member Author

@LoserAntbear Adding lib: [es2015, dom] makes sense to me.

Please send a PR, as long as the new implementation of ex.Promise backed by the _nativePromise behaves in a similar way to the old one (the tests should be a good indicator) that's what we want.

@kamranayub
Copy link
Member

kamranayub commented Jun 13, 2018 via email

@jedeen jedeen modified the milestones: 0.18.0 Release, 0.19.0 Release Jul 20, 2018
@jedeen jedeen modified the milestones: 0.19.0 Release, 0.20.0 Release Oct 13, 2018
@jedeen jedeen modified the milestones: 0.20.0 Release, 0.21.0 Release Dec 11, 2018
@jedeen jedeen modified the milestones: 0.21.0 Release, 0.22.0 Release Dec 21, 2018
@alanag13 alanag13 self-assigned this Mar 16, 2019
@jedeen jedeen modified the milestones: 0.22.0 Release, 0.23.0 Release Apr 6, 2019
@jedeen jedeen modified the milestones: 0.24.0 Release, 0.25.0 Nov 24, 2019
eonarheim added a commit that referenced this issue Nov 14, 2020
Closes #994

## Changes:

- Retarget to es2015 from es5 for browser promises (without polyfill)
- Replace Excalibur `ex.Promise` with built in browser promise
- Refactor loader
- Fix tests, subtle changes needed to be made because callbacks are now on the microtask queue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api change Breaking api change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants