[pbs-devel] [PATCH proxmox-backup 1/2] gui: remove document.execCommand calls

[Dominik Csapak]

On 3/14/24 10:08, Gabriel Goller wrote:
> On Thu Mar 14, 2024 at 8:59 AM CET, Dominik Csapak wrote:
>> hi,
>>
>> just to note: changing to an async function can be rather dangerous,
>> sometimes extjs not only calls the handlers/events/etc. but does things
>> after them (expecting the function to be finished) or waits for the return value.
> 
> Hmm, but does it actually **await** the return value? I'm not a ext.js
> expert but I don't think it is. Anyways it's fine because js promises
> are executed eagerly + promise returned directly.

no it does not awaits anything and thats exactly the problem,
for example (not a real one)

extjs expects a callback foo
after it calls it, it expects foo to be finished to do further work
(e.g. a cleanup after an event handler)

we give an async function as foo, with multiple await calls in there

now it's not guaranteed the callback finished when extjs expects it to

> 
>> (most of the extjs code was from before async/await was a thing in js)
>>
>> so here it seems to work out fine, but we have to be careful with
>> sprinkling async function in the code, otherwise we'll get
>> very unexpected results
>>
>> in general, i'd like to see that mentioned in the commit message
>> why it's ok to do that (no hard feelings though)
>>
>> otherwise LGTM
> 
> What do you think about adding this to the commit message:
> 
>      Making the handler functions async is not a problem, because
>      promises in js are executed eagerly (not lazily) and nothing
>      depends/waits on the result of this handler.

as i mentioned above i don't think that's the reason why it's okay.

it should be okay because extjs executes the handler and does not expect
any result from it, nor does it need to do some work afterwards