From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 984BDB965E for ; Thu, 14 Mar 2024 10:14:06 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 73BFC967A for ; Thu, 14 Mar 2024 10:13:36 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 14 Mar 2024 10:13:32 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 008CF489D9; Thu, 14 Mar 2024 10:13:32 +0100 (CET) Message-ID: Date: Thu, 14 Mar 2024 10:13:31 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Gabriel Goller , Proxmox Backup Server development discussion References: <20240313161801.132483-1-g.goller@proxmox.com> <2524761d-e6b9-4d89-84ef-03d261029f92@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.019 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH proxmox-backup 1/2] gui: remove document.execCommand calls X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Mar 2024 09:14:06 -0000 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