-
Notifications
You must be signed in to change notification settings - Fork 0
test explor #119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
test explor #119
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -172,12 +172,18 @@ class HttpClient { | |
| const controller = new AbortController(); | ||
| const timeoutId = setTimeout(() => controller.abort(), timeout); | ||
|
|
||
| const response = await fetch(fullUrl, { | ||
| ...fetchConfig, | ||
| headers, | ||
| body, | ||
| signal: controller.signal, | ||
| }); | ||
| let response: Response; | ||
| try { | ||
| response = await fetch(fullUrl, { | ||
| ...fetchConfig, | ||
| headers, | ||
| body, | ||
| signal: controller.signal, | ||
| }); | ||
| } catch (fetchError) { | ||
| clearTimeout(timeoutId); | ||
| throw fetchError; | ||
| } | ||
|
|
||
| clearTimeout(timeoutId); | ||
|
|
||
|
|
@@ -317,6 +323,13 @@ class HttpClient { | |
| }; | ||
| } | ||
|
|
||
| if (error.message.includes('Failed to fetch') || error.message.includes('NetworkError')) { | ||
| return { | ||
| code: ERROR_CODES.NETWORK_ERROR, | ||
| message: 'Network connection failed. Please check your internet connection.', | ||
| }; | ||
| } | ||
|
Comment on lines
+326
to
+331
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium 🐛 Bug: Fragile error detection relies on string matching which is locale-dependent and browser-specific The error message matching approach is unreliable because:
- if (error.message.includes('Failed to fetch') || error.message.includes('NetworkError')) {
+ // Check for network errors by error type/name instead of message
+ if (error.name === 'TypeError' || error instanceof TypeError) {
return {
code: ERROR_CODES.NETWORK_ERROR,
message: 'Network connection failed. Please check your internet connection.',
};
}
return {
code: ERROR_CODES.NETWORK_ERROR,
message: error.message || 'Network error occurred',Note: This code block appears to be unreachable since it comes after a generic |
||
|
|
||
| return { | ||
| code: ERROR_CODES.NETWORK_ERROR, | ||
| message: error.message || 'Network error occurred', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import { useEffect, useRef, useCallback } from 'react'; | ||
|
|
||
| export function useThrottle<T extends (...args: unknown[]) => unknown>( | ||
| callback: T, | ||
| delay: number | ||
| ): T { | ||
| const lastRun = useRef<number>(0); | ||
| const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); | ||
|
|
||
| const throttledCallback = useCallback( | ||
| ((...args: Parameters<T>) => { | ||
| const now = Date.now(); | ||
| const timeSinceLastRun = now - lastRun.current; | ||
|
|
||
| if (timeSinceLastRun >= delay) { | ||
| lastRun.current = now; | ||
| return callback(...args); | ||
| } | ||
|
|
||
| if (timeoutRef.current) { | ||
| clearTimeout(timeoutRef.current); | ||
| } | ||
|
|
||
| timeoutRef.current = setTimeout(() => { | ||
| lastRun.current = Date.now(); | ||
| callback(...args); | ||
| }, delay - timeSinceLastRun); | ||
| }) as T, | ||
| [callback, delay] | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| return () => { | ||
| if (timeoutRef.current) { | ||
| clearTimeout(timeoutRef.current); | ||
| } | ||
| }; | ||
| }, []); | ||
|
|
||
| return throttledCallback; | ||
| } | ||
|
Comment on lines
+3
to
+41
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium severity: medium The throttledCallback doesn't include -export function useThrottle<T extends (...args: unknown[]) => unknown>(
- callback: T,
- delay: number
-): T {
- const lastRun = useRef<number>(0);
- const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
-
- const throttledCallback = useCallback(
- ((...args: Parameters<T>) => {
- const now = Date.now();
- const timeSinceLastRun = now - lastRun.current;
-
- if (timeSinceLastRun >= delay) {
- lastRun.current = now;
- return callback(...args);
- }
-
- if (timeoutRef.current) {
- clearTimeout(timeoutRef.current);
- }
-
- timeoutRef.current = setTimeout(() => {
- lastRun.current = Date.now();
- callback(...args);
- }, delay - timeSinceLastRun);
- }) as T,
- [callback, delay]
- );
+export function useThrottle<T extends (...args: unknown[]) => unknown>(
+ callback: T,
+ delay: number
+): T {
+ const lastRun = useRef<number>(0);
+ const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+ const callbackRef = useRef(callback);
+
+ useEffect(() => {
+ callbackRef.current = callback;
+ }, [callback]);
+
+ const throttledCallback = useCallback(
+ ((...args: Parameters<T>) => {
+ const now = Date.now();
+ const timeSinceLastRun = now - lastRun.current;
+
+ if (timeSinceLastRun >= delay) {
+ lastRun.current = now;
+ return callbackRef.current(...args);
+ }
+
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+
+ timeoutRef.current = setTimeout(() => {
+ lastRun.current = Date.now();
+ callbackRef.current(...args);
+ }, delay - timeSinceLastRun);
+ }) as T,
+ [delay]
+ ); |
||
|
|
||
| export function useIdleCallback(callback: () => void, deps: unknown[] = []) { | ||
| const callbackRef = useRef(callback); | ||
|
|
||
| useEffect(() => { | ||
| callbackRef.current = callback; | ||
| }, [callback]); | ||
|
|
||
| useEffect(() => { | ||
| if (typeof requestIdleCallback !== 'undefined') { | ||
| const id = requestIdleCallback(() => { | ||
| callbackRef.current(); | ||
| }); | ||
| return () => cancelIdleCallback(id); | ||
| } else { | ||
| const timeoutId = setTimeout(() => { | ||
| callbackRef.current(); | ||
| }, 1); | ||
| return () => clearTimeout(timeoutId); | ||
| } | ||
| }, deps); | ||
| } | ||
|
Comment on lines
+43
to
+63
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Severity: HIGH This issue appears in 3 locations across 2 files. Each location requires specific attention based on its context. 📍 Affected Locations: 💡 General Guidance: use the effect to re-run on every render if 📋 Specific Suggestions for Each File (Click to expand)
|
||
|
|
||
| export function useIntersectionObserver( | ||
| elementRef: React.RefObject<Element>, | ||
| callback: (isIntersecting: boolean) => void, | ||
| options?: IntersectionObserverInit | ||
| ) { | ||
| useEffect(() => { | ||
| const element = elementRef.current; | ||
| if (!element) return; | ||
|
|
||
| const observer = new IntersectionObserver(([entry]) => { | ||
| callback(entry.isIntersecting); | ||
| }, options); | ||
|
|
||
| observer.observe(element); | ||
|
|
||
| return () => { | ||
| observer.disconnect(); | ||
| }; | ||
| }, [elementRef, callback, options]); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Medium
🐛 Bug: ErrorBoundary placement outside ToastProvider prevents error notifications
The ErrorBoundary wraps the ToastProvider, which means if an error occurs, the toast system won't be available to display error messages. This breaks the user feedback mechanism during errors.
Alternative consideration: If ErrorBoundary needs to catch ToastProvider errors, consider having two ErrorBoundaries - one outside for ToastProvider errors (with basic fallback), and one inside for application errors (with toast notifications).