Fix some incorrect async functions, improve frontend document. (go-gitea#17597)

Author: wxiaoguang

docs/content/doc/developers/guidelines-frontend.md

@@ -39,6 +39,65 @@ We recommend [Google HTML/CSS Style Guide](https://google.github.io/styleguide/h
 6. The backend can pass complex data to the frontend by using `ctx.PageData["myModuleData"] = map[]{}`
 7. Simple pages and SEO-related pages use Go HTML Template render to generate static Fomantic-UI HTML output. Complex pages can use Vue2 (or Vue3 in future).
 
+
+### `async` Functions
+
+Only mark a function as `async` if and only if there are `await` calls 
+or `Promise` returns inside the function.
+
+It's not recommended to use `async` event listeners, which may lead to problems. 
+The reason is that the code after await is executed outside the event dispatch. 
+Reference: https://github.com/github/eslint-plugin-github/blob/main/docs/rules/async-preventdefault.md
+
+If we want to call an `async` function in a non-async context,
+it's recommended to use `const _promise = asyncFoo()` to tell readers
+that this is done by purpose, we want to call the async function and ignore the Promise.
+Some lint rules and IDEs also have warnings if the returned Promise is not handled.
+
+#### DOM Event Listener
+
+```js
+el.addEventListener('click', (e) => {
+  (async () => {
+    await asyncFoo(); // recommended
+    // then we shound't do e.preventDefault() after await, no effect
+  })(); 
+  
+  const _promise = asyncFoo(); // recommended
+
+  e.preventDefault(); // correct
+});
+
+el.addEventListener('async', async (e) => { // not recommended but acceptable
+  e.preventDefault(); // acceptable
+  await asyncFoo();   // skip out event dispath 
+  e.preventDefault(); // WRONG
+});
+```
+
+#### jQuery Event Listener
+
+```js
+$('#el').on('click', (e) => {
+  (async () => {
+    await asyncFoo(); // recommended
+    // then we shound't do e.preventDefault() after await, no effect
+  })();
+
+  const _promise = asyncFoo(); // recommended
+
+  e.preventDefault();  // correct
+  return false;        // correct
+});
+
+$('#el').on('click', async (e) => {  // not recommended but acceptable
+  e.preventDefault();  // acceptable
+  return false;        // WRONG, jQuery expects the returned value is a boolean, not a Promise
+  await asyncFoo();    // skip out event dispath
+  return false;        // WRONG
+});
+```
+
 ### Vue2/Vue3 and JSX
 
 Gitea is using Vue2 now, we plan to upgrade to Vue3. We decided not to introduce JSX to keep the HTML and the JavaScript code separated.

web_src/js/features/clipboard.js

@@ -46,7 +46,7 @@ function fallbackCopyToClipboard(text) {
 }
 
 export default function initGlobalCopyToClipboardListener() {
-  document.addEventListener('click', async (e) => {
+  document.addEventListener('click', (e) => {
     let target = e.target;
     // in case <button data-clipboard-text><svg></button>, so we just search up to 3 levels for performance.
     for (let i = 0; i < 3 && target; i++) {
@@ -58,16 +58,20 @@ export default function initGlobalCopyToClipboardListener() {
       }
       if (text) {
         e.preventDefault();
-        try {
-          await navigator.clipboard.writeText(text);
-          onSuccess(target);
-        } catch {
-          if (fallbackCopyToClipboard(text)) {
+
+        (async() => {
+          try {
+            await navigator.clipboard.writeText(text);
             onSuccess(target);
-          } else {
-            onError(target);
+          } catch {
+            if (fallbackCopyToClipboard(text)) {
+              onSuccess(target);
+            } else {
+              onError(target);
+            }
           }
-        }
+        })();
+
         break;
       }
       target = target.parentElement;

web_src/js/features/notification.js

@@ -3,21 +3,22 @@ const {appSubUrl, csrfToken, notificationSettings} = window.config;
 let notificationSequenceNumber = 0;
 
 export function initNotificationsTable() {
-  $('#notification_table .button').on('click', async function () {
-    const data = await updateNotification(
-      $(this).data('url'),
-      $(this).data('status'),
-      $(this).data('page'),
-      $(this).data('q'),
-      $(this).data('notification-id'),
-    );
-
-    if ($(data).data('sequence-number') === notificationSequenceNumber) {
-      $('#notification_div').replaceWith(data);
-      initNotificationsTable();
-    }
-    await updateNotificationCount();
-
+  $('#notification_table .button').on('click', function () {
+    (async () => {
+      const data = await updateNotification(
+        $(this).data('url'),
+        $(this).data('status'),
+        $(this).data('page'),
+        $(this).data('q'),
+        $(this).data('notification-id'),
+      );
+
+      if ($(data).data('sequence-number') === notificationSequenceNumber) {
+        $('#notification_div').replaceWith(data);
+        initNotificationsTable();
+      }
+      await updateNotificationCount();
+    })();
     return false;
   });
 }
@@ -104,8 +105,8 @@ export function initNotificationCount() {
   }
 
   const fn = (timeout, lastCount) => {
-    setTimeout(async () => {
-      await updateNotificationCountWithCallback(fn, timeout, lastCount);
+    setTimeout(() => {
+      const _promise = updateNotificationCountWithCallback(fn, timeout, lastCount);
     }, timeout);
   };
 

web_src/js/features/repo-graph.js

@@ -48,7 +48,7 @@ export default function initRepoGraphGit() {
   });
   const url = new URL(window.location);
   const params = url.searchParams;
-  const updateGraph = async () => {
+  const updateGraph = () => {
     const queryString = params.toString();
     const ajaxUrl = new URL(url);
     ajaxUrl.searchParams.set('div-only', 'true');
@@ -57,14 +57,15 @@ export default function initRepoGraphGit() {
     $('#rel-container').addClass('hide');
     $('#rev-container').addClass('hide');
     $('#loading-indicator').removeClass('hide');
-
-    const div = $(await $.ajax(String(ajaxUrl)));
-    $('#pagination').html(div.find('#pagination').html());
-    $('#rel-container').html(div.find('#rel-container').html());
-    $('#rev-container').html(div.find('#rev-container').html());
-    $('#loading-indicator').addClass('hide');
-    $('#rel-container').removeClass('hide');
-    $('#rev-container').removeClass('hide');
+    (async () => {
+      const div = $(await $.ajax(String(ajaxUrl)));
+      $('#pagination').html(div.find('#pagination').html());
+      $('#rel-container').html(div.find('#rel-container').html());
+      $('#rev-container').html(div.find('#rev-container').html());
+      $('#loading-indicator').addClass('hide');
+      $('#rel-container').removeClass('hide');
+      $('#rev-container').removeClass('hide');
+    })();
   };
   const dropdownSelected = params.getAll('branch');
   if (params.has('hide-pr-refs') && params.get('hide-pr-refs') === 'true') {

web_src/js/features/repo-legacy.js

@@ -351,6 +351,7 @@ export function initRepository() {
 
     // Edit issue or comment content
     $(document).on('click', '.edit-content', async function (event) {
+      event.preventDefault();
       $(this).closest('.dropdown').find('.menu').toggle('visible');
       const $segment = $(this).closest('.header').next();
       const $editContentZone = $segment.find('.edit-content-zone');
@@ -511,7 +512,6 @@ export function initRepository() {
         $textarea.focus();
         $simplemde.codemirror.focus();
       });
-      event.preventDefault();
     });
 
     initRepoIssueCommentDelete();

web_src/js/features/repo-projects.js

@@ -63,9 +63,7 @@ export default function initRepoProject() {
     return;
   }
 
-  (async () => {
-    await initRepoProjectSortable();
-  })();
+  const _promise = initRepoProjectSortable();
 
   $('.edit-project-board').each(function () {
     const projectHeader = $(this).closest('.board-column-header');

web_src/js/features/stopwatch.js

@@ -82,8 +82,8 @@ export function initStopwatch() {
   }
 
   const fn = (timeout) => {
-    setTimeout(async () => {
-      await updateStopwatchWithCallback(fn, timeout);
+    setTimeout(() => {
+      const _promise = updateStopwatchWithCallback(fn, timeout);
     }, timeout);
   };
 
@@ -122,7 +122,7 @@ async function updateStopwatch() {
   return updateStopwatchData(data);
 }
 
-async function updateStopwatchData(data) {
+function updateStopwatchData(data) {
   const watch = data[0];
   const btnEl = $('.active-stopwatch-trigger');
   if (!watch) {
@@ -135,14 +135,14 @@ async function updateStopwatchData(data) {
     $('.stopwatch-cancel').attr('action', `${issueUrl}/times/stopwatch/cancel`);
     $('.stopwatch-issue').text(`${repo_owner_name}/${repo_name}#${issue_index}`);
     $('.stopwatch-time').text(prettyMilliseconds(seconds * 1000));
-    await updateStopwatchTime(seconds);
+    updateStopwatchTime(seconds);
     btnEl.removeClass('hidden');
   }
 
   return !!data.length;
 }
 
-async function updateStopwatchTime(seconds) {
+function updateStopwatchTime(seconds) {
   const secs = parseInt(seconds);
   if (!Number.isFinite(secs)) return;