addDynamicDependency should be foolproof

[denis-anisimov]

#6602 introduces a new API to add a dependency.
The method accepts JS expression which is supposed to return a Promise.
We may not rely that users will read javadocs and use it properly.
There is a check on the client side that the expression is a Promise.
But that's not enough.

We should either:

• report an error to the server side if it's not a Promise.
• change the client side impl so that it always works regardless of the resulting type.

I think the last approach should be doable via using an async function which awaits for expression execution.
await will automatically wrap the expression into a Promise if it's not already a Promise.
So it will work automatically out of the box.
I have only some doubts about GWT unit tests. It might be an issue to make them work with this change.

[Legioth]

No need to use async and await. You can instead pass the return value from the user expression to Promise.resolve(). If passed a promise, then the method returns that promise. If passed any other value, then the method return a promise that resolves to the original value.

On the other hand, I question the need for additional checks here. If you pass a string with inappropriate contents to e.g. addStyleSheet, then you will also just get a client-side error without any clear indication of what to do about it. Why should we invest time in any additional error handling for this particular way of adding dependencies?

[denis-anisimov]

Why should we invest time in any additional error handling for this particular way of adding dependencies?

I think it's too easy to make a mistake here.
But we can wait for external bug/enhancement report of course.
It's not high priority ticket for sure.

[denis-anisimov]

No need to use async and await. You can instead pass the return value from the user expression to Promise.resolve()

That will require the user expression to be executed synchronously in the browser loop.
Wrapping an expression into a Promise will allow to execute it async as I understand, outside of browser loop.

[Legioth]

That will require the user expression to be executed synchronously in the browser loop.

Promise processing is added to the microtask queue which is purged after the currently running script but before the browser can continue with other tasks such as rendering or processing user events.

I don't see why the current way of running would be a problem assuming the user doesn't do heavy number chunking in the expression. Any (reasonable) way of doing I/O from the user expression will still be asynchronous.

[denis-anisimov]

assuming the user doesn't do heavy number chunking in the expression

That's the point.

I'm just trying to make it "perfect" .
But I'm OK with the assumption.
The ticket is about improving just for the record rather "have to do".