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

Eliminate when.js dependency in distribution #314

Closed
techfg opened this issue Jan 5, 2021 · 1 comment · Fixed by #315
Closed

Eliminate when.js dependency in distribution #314

techfg opened this issue Jan 5, 2021 · 1 comment · Fixed by #315

Comments

@techfg
Copy link
Collaborator

techfg commented Jan 5, 2021

Per @jamietre comment here, to minimize the changes required to support NPM, etc. when.js should be removed and replaced with native promises.

@techfg
Copy link
Collaborator Author

techfg commented Jan 5, 2021

A few notes on approach/benefits of commit:

  1. Goal was to minimize changes to existing code base as a priority so given this, left the "defer" pattern and shimed/polyfilled a native promise solution. Would be better to refactor and move away from defer but wanted to minimize any risk. All tests pass before/after.

  2. There was an existing bug in the when.js library (imagemapster was using 1.2.0, latest version is 3.7.8) that this line of code was subject to. In short, the when.all when passed an array would not actually check easy promise in the array and instead would immediately resolve treating the array itself as a value. This was later fixed in when.js (around 1.5 I think) but IM suffered from it. The added benefit of the changes in this commit are that this gap is closed and should no longer be an issue.

  3. when.js is still used by the testing framework, only removed when.js from src/dist.

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

Successfully merging a pull request may close this issue.

1 participant