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

fix: typescript promise type #160

Merged
merged 7 commits into from
Oct 6, 2022

Conversation

jspears
Copy link
Contributor

@jspears jspears commented Sep 20, 2022

The current tweenable interface is not satisfying typescript definition of a promise. This is a slight annoyance when using the await for a tweenable, as typescript complains it is not a promise. This PR finishes the promise implementation, adds the interface in typescript, and adds a test for good measure.

@jeremyckahn
Copy link
Owner

Awesome! Thank you for the PR @jspears. I’m pretty busy this week, so I may not have time to review this until the weekend. I will follow up here when I can!

Copy link
Owner

@jeremyckahn jeremyckahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great so far @jspears! Thanks for the work you've put into this so far. I noticed one JSDoc description that might need to be modified, but that's it.

src/tweenable.js Show resolved Hide resolved
Comment on lines +563 to +565
* @param {function=} onFulfilled Receives {@link shifty.promisedData} as the
* first parameter.
* @param {function} onRejected Receives {@link shifty.promisedData} as the
* @param {function=} onRejected Receives {@link shifty.promisedData} as the
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this!

src/tweenable.js Outdated Show resolved Hide resolved
Copy link
Owner

@jeremyckahn jeremyckahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix @jspears! I will merge this now and release it as a patch update.

@jeremyckahn jeremyckahn merged commit f0c71d0 into jeremyckahn:develop Oct 6, 2022
@jeremyckahn
Copy link
Owner

This has been released in 2.19.1!

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

3 participants