I feel like this feature was way to easy to implement. This is most likely due to a large amount of functionality and edge cases I'm unaware of. Would love your input/guidance on what else you'd need to see for this to be complete @chriddyp and @alexcjohnson.

I'm not surprised this is pretty easy at this point - there have been a bunch of architectural changes bringing the async server-side callback execution and sync client-side execution closer to parallel. We just hadn't gotten around to trying it - and adding tests to ensure it works as intended. But I wonder if this wouldn't already work just by removing the test for a promise? I see later in the process we do a different test for promises (coming I presume from server-side callbacks) and we await these there:

Did you try this and see it fail? My main concern with blanket converting the synchronous function to async is whether we'll cause multiple page reflows/repaints in the case of multiple concurrent clientside callbacks, which can slow things down or make them look unnecessarily jittery. This is why for example plotly.js makes its own syncOrAsync function to ensure we keep execution as synchronous as possible.

Anyway as I alluded to above the main additional thing this will need is some tests. Perhaps:

• One adding a clientside callback that returns a promise that's known to resolve fairly quickly (don't add any external network requests, those become unreliable in CI tests, just make a small timeout or something) where another callback (clientside or serverside, doesn't matter) uses the output of the first one as an input.
• A pair of tests with a clientside and serverside callback both triggered by the same input, where we engineer it so in one case the serverside callback returns first and in the other case the clientside returns first, and ensure that the page updates first with the faster value before the slow has returned, followed by the slow value. Don't do this with time delays, as those are flaky and slow. Instead have the test code tell the callbacks when they're allowed to return. Server-side this can be done with multiprocessing.Lock. Client-side you should be able to do something like:

function cb(inputVal) {
    return new Promise(function(resolve) {
        window.callbackDone = function(deferredVal) {
            resolve(inputVal + ' - ' + deferredVal);
        }
    })
}

then in the test code, wait for the serverside output to be visible then call:

dash_duo.driver.execute_script("window.callbackDone('the wait is over!')")

@MrTeale thanks very much for kicking this off! Also, don't worry about the existing test failures just yet - we're working on that right now in #2006, hopefully will have that done shortly and then we can merge that in here.

Hey @alexcjohnson! Thanks for your review, was really detailed and helped a tonne to know what direction you wanted to go as the project owner. Quickly to begin, I did try removing the test for a promise, no dice unfortunately.

I've added the tests you suggested and they're passing well. If you (or anyone else) glance across them and sanity check to ensure they do what they're supposed to do. New tests added include:

• clsd015_clientside_chained_callbacks_returning_promise - Tests using an output from a client-side callback (returning a promise) as the input to a server-side component.
• clsd016_serverside_clientside_shared_input_with_promise - Tests a shared input on both client-side (returning a promise) and server-side where the server-side callback finishes first.
• clsd017_clientside_serverside_shared_input_with_promise - Tests a shared input on both client-side (returning a promise) and server-side where the client-side callback finishes first.

Note: I have xfailed clsd005_clientside_fails_when_returning_a_promise as this test will no longer be needed with this addition. I know some projects prefer to remove them, others xfail for posterity.

On the concern of converting a synchronous function to async and the desire to keep it as synchronous as possible, I completely agree. Perhaps we could determine whether the return value is a function or not (currently done here) outside of handleClientside and direct callbacks returning promises to a seperate async version of handleClientside?

Nice! Lovely tests 🏆 The one additional piece that occurs to me, mainly for documentation purposes: I presume it also works if these are declared as async functions, rather than regular functions that return promises? Including using the inline syntax:

app.clientside_callback(
    """
    async function(val) { ... }
    """,
    Output(...), Input(...)
)

I think we can delete clsd005 rather than having it xfail, the new tests cover it all nicely. The git history is enough posterity for me 😉

I did try removing the test for a promise, no dice unfortunately.

OIC, there's the whole reshaping into {id: {prop: val}} as well as deleting callback_context and time tracking. OK - let's leave it as you have it, the parent function __execute is already async so it's likely this won't meaningfully alter the experience for sync clientside callbacks. If this becomes a problem we could try chaining those onto the returned promise and adding a conditional await after the handleClientside call, but maybe that's just unnecessary complexity at this point.

Now that I see it though delaying delete dc.callback_context is a problem - multiple clientside callbacks running simultaneously would trample on each others' contexts. Feels to me like the safest way to handle this would be to move that deletion right after the sync function call, and then document that in an async clientside callback if you need callback_context you must read it in the sync portion of the callback.

I presume it also works if these are declared as async functions, rather than regular functions that return promises? Including using the inline syntax

Yes it does. I have just added a couple more tests to cover this scenario. These two cover both async function() { ... } and new Promise( function (resolve) { ... } styles of async calls.

Feels to me like the safest way to handle this would be to move that deletion right after the sync function call

Definitely agree that, for now, the safest option is to read the callback_context in the synchronous portion. I've added an additional delete dc.callback_context to between the synchronous call to the callback and the check for a promise.

If there's nothing else, I think it's just the docs to be updated and it'll be ready to merge