feat: lazy-load dashboard chart modules with IntersectionObserver#702
feat: lazy-load dashboard chart modules with IntersectionObserver#702
Conversation
- Add src/browser/lazy-loader.ts with IntersectionObserver-based lazy loading - Update src/browser/main.ts: stats-loader stays eager; all 11 chart dashboards converted to dynamic import() triggered on viewport intersection - Add .chart-skeleton CSS to styles.css using existing design tokens - Add tests/lazy-loader.test.js (14 tests, all passing) Reduces initial JS parse cost by deferring Chart.js (~200KB), D3 (~250KB), and PapaParse (~50KB) until their dashboard sections scroll into view. Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
There was a problem hiding this comment.
Pull request overview
This PR introduces viewport-triggered lazy loading for chart-heavy dashboard modules to reduce initial JS download/parse cost and improve TTI, while keeping above-the-fold “stats” eager.
Changes:
- Added a new
initLazyDashboards()helper that usesIntersectionObserverto dynamicallyimport()dashboard modules as their containers near the viewport (with a no-IntersectionObserverfallback). - Refactored
src/browser/main.tsto remove eager/static imports of Chart.js/D3/PapaParse-dependent dashboards and register them as lazy entries. - Added
.chart-skeletonstyling and new Vitest unit tests for the lazy loader behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/browser/lazy-loader.ts |
Adds IntersectionObserver-based lazy dashboard loader with skeleton lifecycle handling and fallback behavior. |
src/browser/main.ts |
Switches chart-heavy dashboards from eager init to lazy dynamic imports; keeps stats eager. |
styles.css |
Adds .chart-skeleton shimmer placeholder styling (with reduced-motion support). |
tests/lazy-loader.test.js |
Adds unit tests covering IO-triggered loading, skeleton lifecycle, dedupe, skipping, and fallback. |
src/browser/lazy-loader.ts
Outdated
| if (typeof IntersectionObserver === 'undefined') { | ||
| // Graceful fallback: load everything immediately | ||
| for (const { containerId, loader } of dashboards) { | ||
| loader().catch((err: unknown) => | ||
| logger.error(`Lazy load failed for #${containerId}:`, err), | ||
| ); | ||
| } | ||
| return; |
There was a problem hiding this comment.
initLazyDashboards() fallback path (when IntersectionObserver is unavailable) calls every dashboard loader() unconditionally. This bypasses the DOM presence check used in the observer path and can trigger dashboard init on pages where that container does not exist (e.g., different routes), causing avoidable errors. Suggestion: mirror the normal path by checking document.getElementById(containerId) before invoking loader() (and optionally apply/remove the skeleton class consistently).
There was a problem hiding this comment.
Fixed in 29043a3. The fallback path now mirrors the observer path: it calls document.getElementById(containerId) before invoking the loader (skipping and logging when the element is absent), and applies/removes the chart-skeleton class around the loader promise — matching the observer path's skeleton lifecycle.
src/browser/main.ts
Outdated
| containerId: 'politician-dashboard', | ||
| loader: async () => { | ||
| await import('./shared/register-globals.js'); | ||
| const { init } = await import('./dashboards/politician-dashboard.js'); | ||
| await init(); | ||
| }, |
There was a problem hiding this comment.
The lazy entry for the politician dashboard uses containerId: 'politician-dashboard', but there is no element with id="politician-dashboard" in politician-dashboard.html (the page uses id="main-content"). With this change the Politician Dashboard module will never be imported/initialized on that page. Update the containerId to an ID that actually exists on the politician dashboard page (or add the missing ID in the HTML as part of this PR).
There was a problem hiding this comment.
Fixed in 29043a3. Added id="politician-dashboard" to the <div class="dashboard-container"> element in politician-dashboard.html. Using id="main-content" wasn't viable since that ID exists on all 14 language home pages as well, which would have triggered the politician dashboard module on every page.
|
@copilot apply changes based on the comments in this thread |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
…containerId - lazy-loader.ts: fallback path now checks getElementById before invoking loader (mirrors IO path), applies/removes chart-skeleton class consistently - politician-dashboard.html: add id="politician-dashboard" to dashboard-container div so the IntersectionObserver can find it (page used id="main-content" on <main> which exists on all pages) - tests/lazy-loader.test.js: add tests for fallback DOM-skip and skeleton lifecycle; add loader call assertion to skeleton test Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
src/browser/main.ts
Outdated
| // Eager: stats loader populates hero metrics — no chart libraries needed | ||
| try { | ||
| await initStats(); | ||
| logger.debug('✓ stats initialized'); | ||
| } catch (error) { | ||
| logger.error('✗ stats failed:', error); | ||
| } | ||
|
|
||
| const succeeded = results.filter((r) => r.status === 'fulfilled').length; | ||
| const failed = results.filter((r) => r.status === 'rejected').length; | ||
| const elapsed = (performance.now() - start).toFixed(0); | ||
| // Lazy: chart-heavy dashboards load only when their section enters the viewport | ||
| initLazyDashboards(LAZY_DASHBOARDS); |
There was a problem hiding this comment.
initLazyDashboards(LAZY_DASHBOARDS) is only registered after awaiting initStats(). Since initStats() does async I/O (CSV fetch/parse), this can delay setting up the IntersectionObserver and postpone (or miss) early lazy-load triggers for dashboards that are already in/near the viewport on initial render. Consider registering initLazyDashboards() before awaiting initStats() (or running them in parallel) so lazy dashboards can start prefetching as soon as DOMContentLoaded fires.
There was a problem hiding this comment.
Fixed in b166ec6. initLazyDashboards(LAZY_DASHBOARDS) is now called before await initStats(), so the IntersectionObserver is live before any async I/O can delay it.
src/browser/main.ts
Outdated
| const LAZY_DASHBOARDS: LazyDashboard[] = [ | ||
| { | ||
| containerId: 'coalition-status', | ||
| loader: async () => { | ||
| await import('./shared/register-globals.js'); | ||
| const { init } = await import('./dashboards/coalition-loader.js'); | ||
| await init(); | ||
| }, |
There was a problem hiding this comment.
The LAZY_DASHBOARDS registry repeats the same await import('./shared/register-globals.js') + const { init } = await import(...) pattern in every loader. This duplication makes it easy for future edits to drift (e.g., forgetting to load globals for one dashboard). Consider extracting a small helper (e.g., a wrapper that loads globals once and then imports/runs init) to centralize the pattern.
There was a problem hiding this comment.
Fixed in b166ec6. Extracted a loadDashboard(moduleLoader) helper that handles the register-globals import and init() call. All 11 lazy entries now use it — the repeated 3-line pattern is gone.
|
@copilot apply changes based on the comments in this thread |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| // Map element → loader for O(1) lookup inside the observer callback | ||
| const pending = new Map<Element, () => Promise<void>>(); | ||
|
|
||
| const observer = new IntersectionObserver((entries: IntersectionObserverEntry[]) => { | ||
| for (const entry of entries) { | ||
| if (!entry.isIntersecting) continue; | ||
|
|
||
| const el = entry.target as HTMLElement; | ||
| observer.unobserve(el); | ||
|
|
||
| const loaderFn = pending.get(el); | ||
| if (!loaderFn) continue; | ||
| pending.delete(el); | ||
|
|
||
| // Show skeleton while the module is downloading / initialising | ||
| el.classList.add(CHART_SKELETON_CLASS); | ||
|
|
||
| loaderFn() | ||
| .then(() => { | ||
| el.classList.remove(CHART_SKELETON_CLASS); | ||
| logger.debug(`✓ lazy loaded #${el.id}`); | ||
| }) | ||
| .catch((err: unknown) => { | ||
| el.classList.remove(CHART_SKELETON_CLASS); | ||
| logger.error(`✗ lazy load failed #${el.id}:`, err); | ||
| }); | ||
| } | ||
| }, options); | ||
|
|
||
| for (const { containerId, loader } of dashboards) { | ||
| const el = document.getElementById(containerId); | ||
| if (!el) { | ||
| logger.debug(`Lazy loader: #${containerId} not in DOM, skipping`); | ||
| continue; | ||
| } | ||
| pending.set(el, loader); | ||
| observer.observe(el); | ||
| } |
There was a problem hiding this comment.
initLazyDashboards() creates an IntersectionObserver but doesn’t keep any reference to it outside the function. After initLazyDashboards() returns, the observer can be garbage-collected in some browsers/conditions, which would stop observations and prevent dashboards from ever loading. Consider retaining the observer (e.g., module-level variable) and/or returning a handle (observer or cleanup function) so its lifetime is guaranteed and it can be disconnected if needed.
There was a problem hiding this comment.
Fixed in 9341bd1. initLazyDashboards() now returns IntersectionObserver | undefined (undefined in the no-IO fallback path). The caller holds the reference, preventing GC. The docstring documents both the return type and when undefined is returned.
| // Show skeleton while the module is downloading / initialising | ||
| el.classList.add(CHART_SKELETON_CLASS); | ||
|
|
||
| loaderFn() | ||
| .then(() => { | ||
| el.classList.remove(CHART_SKELETON_CLASS); | ||
| logger.debug(`✓ lazy loaded #${el.id}`); | ||
| }) | ||
| .catch((err: unknown) => { | ||
| el.classList.remove(CHART_SKELETON_CLASS); | ||
| logger.error(`✗ lazy load failed #${el.id}:`, err); | ||
| }); |
There was a problem hiding this comment.
In both the IntersectionObserver path and the fallback path, loader()/loaderFn() is assumed to always return a Promise and never throw synchronously. If a loader throws before returning (e.g., due to a coding error), the skeleton class won’t be removed and the error won’t be logged by the .catch() handler. Wrapping the invocation in Promise.resolve() (or a try/catch that rethrows as a rejected Promise) would make the skeleton lifecycle and error logging robust to sync exceptions.
There was a problem hiding this comment.
Fixed in 9341bd1. Both the IO path and fallback path now wrap loader invocations in Promise.resolve().then(() => loader()). A synchronous throw inside a loader becomes a rejected promise, which is caught by the existing .catch() handler — ensuring the skeleton class is always removed and the error is always logged.
|
@copilot apply changes based on the comments in this thread |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
- lazy-loader.ts: return IntersectionObserver | undefined from initLazyDashboards() so callers hold a reference preventing GC. Wrap both IO-path and fallback loader calls in Promise.resolve().then() so synchronous throws become rejections, guaranteeing the skeleton class is always removed and errors are always logged. - tests/lazy-loader.test.js: add flushPromises() helper (setTimeout macro-task boundary) to fully drain the microtask queue regardless of promise chain depth. Update 5 existing skeleton-lifecycle tests that were checking synchronous loader dispatch (now async). Add 4 new tests: observer return value (IO + fallback paths), sync-throw skeleton cleanup (IO + fallback paths). Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Done in 9341bd1. Both issues from the review are addressed: observer GC prevention (return value) and sync-throw robustness ( |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| // Register lazy dashboards immediately — IntersectionObserver must be live | ||
| // before initStats()'s async I/O so containers already in/near the viewport | ||
| // on initial render are not missed. | ||
| initLazyDashboards(LAZY_DASHBOARDS); | ||
|
|
There was a problem hiding this comment.
initLazyDashboards() returns the created IntersectionObserver specifically so the caller can retain a reference (preventing GC). Here the return value is ignored, which can allow the observer to be garbage-collected and stop firing, leaving lazy dashboards never loading in some environments. Store the returned observer (even if unused) for the lifetime of the page, or keep it in module scope and optionally disconnect it on teardown/navigation.
There was a problem hiding this comment.
Fixed in 21e2600. Added a module-level _lazyObserver variable in main.ts; initLazyDashboards() return value is now assigned to it, keeping the IntersectionObserver alive for the page lifetime.
|
@copilot apply changes based on the comments in this thread and Test Dashboard / Dashboard Tests (cypress/e2e/dashboards.cy.js) (pull_request) |
…ess test
- main.ts: add module-level `_lazyObserver` variable so the
IntersectionObserver returned by initLazyDashboards() is retained for
the page lifetime, preventing GC from stopping observations (comment
2869074903).
- cypress/e2e/politician-dashboard.cy.js: replace one-shot
`cy.window().then()` check (which could never see a lazily-loaded
window.Chart) with `cy.get('#politician-dashboard').scrollIntoView()`
+ `cy.window().its('Chart').should('exist')`, which scrolls the
container into the IntersectionObserver viewport trigger zone and then
retries until register-globals sets window.Chart — fixing the CI
failure in Test Dashboard / Dashboard Tests
(cypress/e2e/politician-dashboard.cy.js).
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Done in 21e2600. Two fixes applied:
|
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
src/browser/lazy-loader.tswith IntersectionObserver-based lazy loading.chart-skeletonCSS tostyles.csssrc/browser/main.tsto use lazy loading for all dashboards (except stats-loader which stays eager)tests/lazy-loader.test.jswith unit tests for the lazy loaderlazy-loader.ts: check DOM presence and apply skeleton class consistentlyid="politician-dashboard"topolitician-dashboard.htmlsocontainerIdresolves correctlyinitLazyDashboards()beforeawait initStats()so IO does not delay observer setuploadDashboard()helper to remove repeated register-globals + init patternlazy-loader.tsdocstring: remove inaccurate SSR claim from fallback description<script>tag frompolitician-dashboard.htmlIntersectionObserver | undefinedfrominitLazyDashboards()to prevent GCPromise.resolve().then()for sync-throw safety_lazyObserverinmain.tsso GC cannot collect it (comment 2869074903)expect(win.Chart).to.exist→cy.get('#politician-dashboard').scrollIntoView()+cy.window().its('Chart').should('exist')(CI failure fix)Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.