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

tokarchuk/v1.17
wxiaoguang 3 years ago committed by GitHub
parent 0db7a32b92
commit 7f802631c5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 59
      docs/content/doc/developers/guidelines-frontend.md
  2. 6
      web_src/js/features/clipboard.js
  3. 9
      web_src/js/features/notification.js
  4. 5
      web_src/js/features/repo-graph.js
  5. 2
      web_src/js/features/repo-legacy.js
  6. 4
      web_src/js/features/repo-projects.js
  7. 10
      web_src/js/features/stopwatch.js

@ -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.

@ -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,6 +58,8 @@ export default function initGlobalCopyToClipboardListener() {
}
if (text) {
e.preventDefault();
(async() => {
try {
await navigator.clipboard.writeText(text);
onSuccess(target);
@ -68,6 +70,8 @@ export default function initGlobalCopyToClipboardListener() {
onError(target);
}
}
})();
break;
}
target = target.parentElement;

@ -3,7 +3,8 @@ const {appSubUrl, csrfToken, notificationSettings} = window.config;
let notificationSequenceNumber = 0;
export function initNotificationsTable() {
$('#notification_table .button').on('click', async function () {
$('#notification_table .button').on('click', function () {
(async () => {
const data = await updateNotification(
$(this).data('url'),
$(this).data('status'),
@ -17,7 +18,7 @@ export function initNotificationsTable() {
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);
};

@ -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,7 +57,7 @@ export default function initRepoGraphGit() {
$('#rel-container').addClass('hide');
$('#rev-container').addClass('hide');
$('#loading-indicator').removeClass('hide');
(async () => {
const div = $(await $.ajax(String(ajaxUrl)));
$('#pagination').html(div.find('#pagination').html());
$('#rel-container').html(div.find('#rel-container').html());
@ -65,6 +65,7 @@ export default function initRepoGraphGit() {
$('#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') {

@ -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();

@ -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');

@ -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;

Loading…
Cancel
Save