Wrong async/await/preventDefault usage

[wxiaoguang]

This is an issue for discussion of frontend refactoring. @lafriks @silverwind

The code https://github.com/go-gitea/gitea/blob/main/web_src/js/features/repo-legacy.js#L350 works like this:

$(document).on('click', '.edit-content', async function (event) {
    await createDropzone();
    event.preventDefault();
});

// equal to 
$(document).on('click', '.edit-content', function (event) {
    return Promise.resolve(createDropzone).then(() => {
        event.preventDefault();
    });
});

The usage of async/await is incorrect. The preventDefault won't have effect because the event has been processed synchronously before (when the listener returned).

So, that's why I insist to clean up most inconsistent await/async in the code base, only use them when it is necessary.

• Improve async/await usage, and sort init calls in index.js #17386

We should do our best to make everything work as expected.

[silverwind]

Yes, this preventDefault won't work, that is certainly an oversight and there even seems to be an eslint plugin which we could potentially use to detect such issues.

My point thought is that anything that involved loading of additional JS chunks (await import()) should actually be awaited on because subsequent code might depend on the import actually being done loading, just firing off a Promise without awaiting it should only be done sparingly, if at all.

[wxiaoguang]

await import() is the correct usage and I keep them during refactoring, and we encourage to use them

Including await $.ajax()..., they are all good usages.

However the loading should only affect the code which uses the modules. We do not need to do await in global init stage, just leave these async/await calls to separate modules.

Generally speaking I think the code I refactored is on the right way.

[wxiaoguang]

ps: the methods mentioned in "Examples of correct code for this rule" of https://github.com/github/eslint-plugin-github/blob/main/docs/rules/async-preventdefault.md , are the methods I used during refactoring.

For exmple, this one: https://github.com/go-gitea/gitea/pull/17386/files#diff-1b029e915c1ca8d9b0d09ef0ce86797a36fbdca25ab233f179c14987a3bb0a05R137

(async () => {
...
})();

[silverwind]

Yeah, async IIFE seems to be the most elegant way to split a function into sync/async parts.

[wxiaoguang]

Can you help to review the PR #17386 again? I hope it can be approved soon to continue other works.

At least, it doesn't make thing worse than before.