tenders lazy-loading#601
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a small lazy-loading utility layer (intersection/idle triggers + dynamic component loader) and starts applying it to the tenders notifications UI.
Changes:
- Introduces
LazyLoadBoundary,useIntersectionObserver, anduseIdleCallbackhooks to support viewport/interaction/idle preloading. - Adds a
DynamicLazyhelper forReact.lazy+ preload support, plus an examples file. - Updates the tenders client page to wrap the list in
Suspenseand switchesTendersListto a default import/export.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| components/lazy/useIntersectionObserver.ts | New hook for viewport-based mount triggering via IntersectionObserver. |
| components/lazy/useIdleCallback.ts | New hook intended to schedule work during browser idle time. |
| components/lazy/LazyLoadBoundary.tsx | New wrapper component providing trigger-based lazy mounting + optional preloading. |
| components/lazy/DynamicLazy.tsx | New helper to create lazy components with an explicit .preload() method. |
| components/lazy/examples.tsx | Adds example usage of the lazy-loading helpers. |
| app/[locale]/notifications/tenders/TendersPageClient.tsx | Attempts to “lazy-load” the tenders list by adding Suspense and changing import style. |
| app/[locale]/notifications/tenders/TendersList.tsx | Tries to add a default export for TendersList. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function useIntersectionObserver<T extends Element>( | ||
| options: UseIntersectionObserverOptions = {} | ||
| ): [React.RefObject<T>, boolean] { | ||
| const { |
There was a problem hiding this comment.
The return type uses React.RefObject<T> but this file doesn’t import the React namespace/types, which will fail under TS strict ("Cannot find namespace 'React'"). Prefer importing type RefObject from react and returning RefObject<T | null>, or add import type React from 'react' and ensure the ref type matches null initialization.
| ): [React.RefObject<T>, boolean] { | ||
| const { | ||
| root = null, | ||
| rootMargin = '0px', | ||
| threshold = 0, | ||
| freezeOnceVisible = true, | ||
| } = options; | ||
| const ref = useRef<T>(null); |
There was a problem hiding this comment.
useRef<T>(null) is not type-safe with strictNullChecks (enabled in tsconfig) because null isn’t assignable to T. Use useRef<T | null>(null) (and reflect that in the returned ref type) or initialize with null! if you intentionally want a non-null ref contract.
| ): [React.RefObject<T>, boolean] { | |
| const { | |
| root = null, | |
| rootMargin = '0px', | |
| threshold = 0, | |
| freezeOnceVisible = true, | |
| } = options; | |
| const ref = useRef<T>(null); | |
| ): [React.RefObject<T | null>, boolean] { | |
| const { | |
| root = null, | |
| rootMargin = '0px', | |
| threshold = 0, | |
| freezeOnceVisible = true, | |
| } = options; | |
| const ref = useRef<T | null>(null); |
| export function useIdleCallback( | ||
| cb: () => void, | ||
| _deps: React.DependencyList = [] | ||
| ) { |
There was a problem hiding this comment.
React.DependencyList is referenced but React isn’t imported, which will fail type-checking. Import type DependencyList from react (or import type React from 'react') and use that type instead.
| interface GalleryCarouselProps extends React.JSX.IntrinsicAttributes { | ||
| carouselProps?: CarouselProps; | ||
| children: React.ReactNode[]; | ||
| className?: string; | ||
| itemClassName?: string; | ||
| } | ||
|
|
||
| const GalleryBelowFold = DynamicLazy<GalleryCarouselProps>(() => | ||
| import('../carousels/gallery').then((mod) => ({ | ||
| default: mod.GalleryCarousel, | ||
| })) | ||
| ); |
There was a problem hiding this comment.
GalleryBelowFold is declared but never used, which will trigger @typescript-eslint/no-unused-vars warnings. Either use it in an exported example component or remove the unused constant (and any now-unused types/imports).
|
|
||
| import type { CarouselProps } from '~/components/carousels'; | ||
|
|
||
| // import type { CarouselProps } from '~/components/carousels'; |
There was a problem hiding this comment.
There’s a duplicated (commented-out) CarouselProps import left in the file. Please remove commented-out imports to keep the examples clean and avoid confusion.
| // import type { CarouselProps } from '~/components/carousels'; |
| export function useIdleCallback( | ||
| cb: () => void, | ||
| _deps: React.DependencyList = [] | ||
| ) { |
There was a problem hiding this comment.
_deps is accepted but ignored, and the effect only depends on cb. Since callers typically pass an inline callback, this ends up scheduling/canceling an idle callback on every render. Use the provided deps list as the effect dependencies and/or keep the latest callback in a ref so you don’t resubscribe every render.
|
|
||
| // Removed stray comment | ||
| import { Button } from '~/components/buttons'; | ||
| import { |
There was a problem hiding this comment.
Please remove the // Removed stray comment note—this kind of meta-comment adds noise and isn’t useful for future readers.
| // Removed duplicate import statement for Link | ||
| // import Link from 'next/link'; | ||
| return ( |
There was a problem hiding this comment.
Please remove the commented-out Link import and the note about removing it. Commented-out code and meta-notes like this tend to rot and make the file harder to scan.
| } | ||
| } | ||
| }; | ||
| }, [cb]); | ||
| } |
There was a problem hiding this comment.
The effect depends only on [cb] (line with }, [cb]);), which will retrigger when an inline callback is recreated each render, cancelling/rescheduling the idle task repeatedly. Consider depending on the explicit deps list instead and invoking the latest callback via a ref.
| <Suspense fallback={<div>Loading tenders...</div>}> | ||
| <TendersList | ||
| tenders={displayedTenders} | ||
| locale={locale} | ||
| canManage={canManage} |
There was a problem hiding this comment.
<Suspense> won’t provide lazy-loading here because TendersList is imported synchronously and doesn’t suspend. If the intent is lazy-loading, switch TendersList to a React.lazy(() => import('./TendersList'))/Next dynamic() import; otherwise drop the Suspense wrapper.
No description provided.