feat: make cursor waiting when route #419

bzy-debug · 2025-01-13T07:35:39Z

No description provided.

peter-jerry-ye-code-review · 2025-01-13T07:35:58Z

‼️ This code review is generated by a bot. Please verify the content before trusting it.

Here are some observations and potential issues from the provided git diff:

Cursor Wait Class Management:
- The code adds a cursor-wait class to the <html> element before making a fetch request and removes it after the request completes. However, if the fetch request fails (e.g., due to a network error), the cursor-wait class will not be removed, leaving the cursor in a "wait" state indefinitely. This could lead to a poor user experience.
- Suggestion: Wrap the fetch call in a try-finally block to ensure the cursor-wait class is removed even if an error occurs:
```
async function getRouteData(href: string) {
  const url = getRouteDataHref(href);
  document.querySelector("html")!.classList.add("cursor-wait");
  try {
    const res = await fetch(url);
    return await res.json();
  } finally {
    document.querySelector("html")!.classList.remove("cursor-wait");
  }
}
```

Error Handling:

The code does not handle potential errors from the fetch call or the res.json() call. If the request fails or the response is not valid JSON, the function will throw an unhandled error.

Suggestion: Add error handling to gracefully manage such cases:

async function getRouteData(href: string) {
  const url = getRouteDataHref(href);
  document.querySelector("html")!.classList.add("cursor-wait");
  try {
    const res = await fetch(url);
    if (!res.ok) {
      throw new Error(`HTTP error! status: ${res.status}`);
    }
    return await res.json();
  } catch (error) {
    console.error("Error fetching route data:", error);
    throw error; // Re-throw the error if needed
  } finally {
    document.querySelector("html")!.classList.remove("cursor-wait");
  }
}

Non-Null Assertion:
- The code uses the non-null assertion operator (!) when querying the <html> element (document.querySelector("html")!). While this is likely safe in most cases, it assumes the <html> element always exists, which might not be true in all environments or edge cases.
- Suggestion: Add a check to ensure the <html> element exists before manipulating its class list:
```
const htmlElement = document.querySelector("html");
if (htmlElement) {
  htmlElement.classList.add("cursor-wait");
}
// Similarly for removing the class
```

These changes would make the code more robust and user-friendly.

feat: make cursor waiting when route

2976781

bzy-debug merged commit d189bf5 into main Jan 13, 2025
2 of 4 checks passed

bzy-debug deleted the zhiyuan/cursour-wait-route branch January 13, 2025 07:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: make cursor waiting when route #419

feat: make cursor waiting when route #419

bzy-debug commented Jan 13, 2025

peter-jerry-ye-code-review bot commented Jan 13, 2025

feat: make cursor waiting when route #419

feat: make cursor waiting when route #419

Conversation

bzy-debug commented Jan 13, 2025

peter-jerry-ye-code-review bot commented Jan 13, 2025