Fix delay to return delayed function result for use

[ryoarmanda]

What is the purpose of this pull request?

• Documentation update
• Bug fix
• Feature addition or enhancement
• Code maintenance
• Others, please explain:

Resolves #1537.

Overview of changes:

The part of the delay function that caused the issue is specifically this:

markbind/packages/core/src/utils/delay.js

Lines 25 to 34 in 5c826ce

	if (waitingPromise === null) {
	waitingPromise = Promise.all([Promise.delay(wait), runningPromise])
	.finally(() => {
	runningPromise = waitingPromise || Promise.resolve();
	waitingPromise = null;
	const funcPromise = func.apply(context, [pendingArgs]);
	pendingArgs = [];
	return funcPromise;
	});
	}

Notice the use of finally to start calling the delayed function. finally is a bit finicky, through some experiments, turns out the return of finally cannot be used in the next then/after await (in fact, finally does not disrupt the return value of the promise, ref: here) so the return funcPromise is effectively unable to be used.

Changing finally to then fixes the issue, and I believe still preserves the behaviour of the previous implementation.

Reproduction and fixed demos is available here: https://codesandbox.io/s/cool-sid-25hmb. Check the console tab on the bottom right part of the page and refresh the "browser" on the right panel to test it over and over. Can play with flowOriginal and flowFixed for the original and fixed implementation, the argument is the number of calls of the delayed doWork function (can try 1 first to understand the behaviours, I put 2 to test consistency of the approach).

Notice the condition of the image in the issue is reproduced when using flowOriginal and all the doWork calls get resolved.

Anything you'd like to highlight / discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Fix delay to return delayed function result for use

---

Checklist: ☑️

• Updated the documentation for feature additions and enhancements
• Added tests for bug fixes or features
• Linked all related issues
• No blatantly unrelated changes
• Pinged someone for a review!

[jonahtanjz]

Nice work and it looks good 👍 Just one question below.

[jonahtanjz]

One question: Will this promise ever be rejected? If it does will this cause a change in behaviour (callback in .then will not run in this case but in .finally will always run regardless)?

[ryoarmanda]

I don't think there is a case for that promise to be rejected. My observation is that waitingPromise is initially a Promise.all of a Promise.delay and a Promise.resolve, and based on the callback, subsequent waitingPromise will be either:

• Similar to the original (Promise.all of Promise.delay and Promise.resolve; or
• A Promise.all of Promise.delay and the previous waitingPromise

waitingPromise essentially just boils down to the behaviour of Promise.delay and Promise.resolve. Both have no chance of rejection: Promise.delay without second argument returns undefined on resolve, the same with Promise.resolve without argument. Thus, waitingPromise will never be rejected.

[jonahtanjz]

Alright :) Thanks for the clarification and detailed explanation 👍

[jonahtanjz]

LGTM :)