]> source.dussan.org Git - gitea.git/commitdiff
Fix some incorrect async functions, improve frontend document. (#17597)
authorwxiaoguang <wxiaoguang@gmail.com>
Fri, 12 Nov 2021 12:37:45 +0000 (20:37 +0800)
committerGitHub <noreply@github.com>
Fri, 12 Nov 2021 12:37:45 +0000 (20:37 +0800)
docs/content/doc/developers/guidelines-frontend.md
web_src/js/features/clipboard.js
web_src/js/features/notification.js
web_src/js/features/repo-graph.js
web_src/js/features/repo-legacy.js
web_src/js/features/repo-projects.js
web_src/js/features/stopwatch.js

index f30b0d1cbd8864f9f3cc585d56a881370da99823..c937cfb7b4dd3c9722928ef6f35925f768011c22 100644 (file)
@@ -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.
index 8d28b4e281c5fe06e23dc9527662fc2d3b93e966..89aface93aca523638262ced6e5841649b484d26 100644 (file)
@@ -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;
index f4c31c5ede9218bf606e372e885cf678eff65422..1e483b16c8bab8ff9a4f4f618a1506095717d119 100644 (file)
@@ -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);
   };
 
index 007cf9b38dafc77c4fb7a43c1948524544d3fd40..73bde5facd69fbc0446c48f8fe04eba1b2dab2f1 100644 (file)
@@ -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') {
index f4a8c0cf3e9fee1ddeb819187121399f5ad9e0dc..8945360cd5804950a9fcb084d312ea04dadd5e2e 100644 (file)
@@ -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();
index 995b971be55203db8c9a9bc51adcbeae3793f59e..270d54671304da1740f305c4dda0da0a29b327ba 100644 (file)
@@ -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');
index ff3edaf8cc8ad02fc6c1f7fdd041e385a705ad93..f6185f7ff73acf0691dc2868b3aee4a8ced6097e 100644 (file)
@@ -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;