-
-
Notifications
You must be signed in to change notification settings - Fork 237
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
feat: Support promise in watch util #825
Conversation
Implements pmndrs#507
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 554fc73:
|
Thanks for working on it. Been busy, but hopefully, I'll be able to take a look sometime soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for working on it and sorry for the delay.
I'm not super confident with the watch code, but if we have good test coverage, it will be mergeable.
Just a small request is to change the diff in this PR as minimal as possible.
}) | ||
|
||
vi.runAllTimers() | ||
await waitPromise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this instance yes - because whilst the timers will be run - the code is still synchronous and won't wait for the other scheduled tasks to finish - like the promise itself which is scheduled as a macro task so it would resolve after the current function call. It's similar to why you do await Promise.resolve()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yeah. Right. My oversight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
Related Issues or Discussions
#507
Summary
Updates the watch util to support a promise callback. Proxies are subscribed to immediately to improve the time to first callback. This way a proxy can be subscribed to before the promise finishes fully.
Check List
yarn run prettier
for formatting code and docs