fix: harden frontend hook lifecycle cleanup

This commit is contained in:
Graham McIntire 2026-03-23 12:28:41 -05:00
parent 0b7b421c52
commit e048be18d1
No known key found for this signature in database
3 changed files with 100 additions and 16 deletions

View file

@ -6,7 +6,7 @@ import { LiveSocket } from "phoenix_live_view";
declare global { declare global {
interface Window { interface Window {
topbar: { topbar?: {
config: (opts: { barColors: Record<number, string>; shadowColor: string }) => void; config: (opts: { barColors: Record<number, string>; shadowColor: string }) => void;
show: (delay?: number) => void; show: (delay?: number) => void;
hide: () => void; hide: () => void;
@ -47,6 +47,12 @@ interface HookDef {
[key: string]: unknown; [key: string]: unknown;
} }
interface DeferredHookContext {
el?: HTMLElement;
__appDestroyed?: boolean;
__chartBundleCheckCount?: number;
}
const Hooks: Record<string, HookDef> = {}; const Hooks: Record<string, HookDef> = {};
// Singleton map bundle loader — ensures the script is only appended once // Singleton map bundle loader — ensures the script is only appended once
@ -79,7 +85,9 @@ function loadMapBundle(callback: () => void) {
script.onerror = () => { script.onerror = () => {
console.error("Failed to load map bundle"); console.error("Failed to load map bundle");
mapBundleLoading = false; mapBundleLoading = false;
const cbs = mapBundleCallbacks;
mapBundleCallbacks = []; mapBundleCallbacks = [];
cbs.forEach((cb) => cb());
}; };
document.head.appendChild(script); document.head.appendChild(script);
} else { } else {
@ -87,30 +95,52 @@ function loadMapBundle(callback: () => void) {
} }
} }
function isHookActive(context: DeferredHookContext): boolean {
return !context.__appDestroyed && !!context.el?.isConnected;
}
// Map hooks - load map bundle when needed // Map hooks - load map bundle when needed
const originalMapMounted = MapAPRSMap.mounted; const originalMapMounted = MapAPRSMap.mounted;
const originalMapDestroyed = MapAPRSMap.destroyed;
Hooks.APRSMap = { Hooks.APRSMap = {
...MapAPRSMap, ...MapAPRSMap,
mounted() { mounted() {
const self = this; const self = this as DeferredHookContext;
self.__appDestroyed = false;
loadMapBundle(() => { loadMapBundle(() => {
if (originalMapMounted) { if (originalMapMounted && isHookActive(self)) {
originalMapMounted.call(self); originalMapMounted.call(self);
} }
}); });
}, },
destroyed() {
const self = this as DeferredHookContext;
self.__appDestroyed = true;
if (originalMapDestroyed) {
originalMapDestroyed.call(self);
}
},
}; };
const originalInfoMapDestroyed = InfoMap.destroyed;
Hooks.InfoMap = { Hooks.InfoMap = {
...InfoMap, ...InfoMap,
mounted() { mounted() {
const self = this; const self = this as DeferredHookContext;
self.__appDestroyed = false;
loadMapBundle(() => { loadMapBundle(() => {
if (InfoMap.mounted) { if (InfoMap.mounted && isHookActive(self)) {
InfoMap.mounted.call(self); InfoMap.mounted.call(self);
} }
}); });
}, },
destroyed() {
const self = this as DeferredHookContext;
self.__appDestroyed = true;
if (originalInfoMapDestroyed) {
originalInfoMapDestroyed.call(self);
}
},
}; };
// Chart hooks - load chart bundle when needed // Chart hooks - load chart bundle when needed
@ -120,15 +150,27 @@ Object.keys(WeatherChartHooks).forEach((hookName) => {
Hooks[hookName] = { Hooks[hookName] = {
...originalHook, ...originalHook,
mounted() { mounted() {
const self = this; const self = this as DeferredHookContext;
self.__appDestroyed = false;
self.__chartBundleCheckCount = 0;
if (window.VendorLoader && !window.chartBundleLoaded) { if (window.VendorLoader && !window.chartBundleLoaded) {
window.VendorLoader.loadCharts(); window.VendorLoader.loadCharts();
const checkChartLoaded = () => { const checkChartLoaded = () => {
if (!isHookActive(self)) {
return;
}
if ((self.__chartBundleCheckCount || 0) >= 100) {
console.error(`Timed out waiting for chart bundle for ${hookName}`);
return;
}
if (window.Chart) { if (window.Chart) {
if (originalHook.mounted) { if (originalHook.mounted) {
originalHook.mounted.call(self); originalHook.mounted.call(self);
} }
} else { } else {
self.__chartBundleCheckCount = (self.__chartBundleCheckCount || 0) + 1;
setTimeout(checkChartLoaded, 50); setTimeout(checkChartLoaded, 50);
} }
}; };
@ -139,6 +181,13 @@ Object.keys(WeatherChartHooks).forEach((hookName) => {
} }
} }
}, },
destroyed() {
const self = this as DeferredHookContext;
self.__appDestroyed = true;
if (originalHook.destroyed) {
originalHook.destroyed.call(self);
}
},
}; };
}); });
@ -194,12 +243,14 @@ const liveSocket = new LiveSocket("/live", Socket, {
}); });
// Show progress bar on live navigation and form submits // Show progress bar on live navigation and form submits
topbar.config({ if (topbar) {
barColors: { 0: "#29d" }, topbar.config({
shadowColor: "rgba(0, 0, 0, .3)", barColors: { 0: "#29d" },
}); shadowColor: "rgba(0, 0, 0, .3)",
window.addEventListener("phx:page-loading-start", (_info) => topbar.show(100)); });
window.addEventListener("phx:page-loading-stop", (_info) => topbar.hide()); window.addEventListener("phx:page-loading-start", (_info) => topbar.show(100));
window.addEventListener("phx:page-loading-stop", (_info) => topbar.hide());
}
// Handle connection draining reconnect events // Handle connection draining reconnect events
window.addEventListener("phx:reconnect", ((e: CustomEvent<{ delay?: number }>) => { window.addEventListener("phx:reconnect", ((e: CustomEvent<{ delay?: number }>) => {

View file

@ -27,6 +27,9 @@ interface ChartHookContext extends Hook {
chart?: Chart; chart?: Chart;
themeChangeHandler?: () => void; themeChangeHandler?: () => void;
renderChart: () => void; renderChart: () => void;
isDestroyed?: boolean;
chartLoadRetryTimer?: ReturnType<typeof setTimeout> | null;
chartLoadRetryCount?: number;
} }
// Type for weather history data // Type for weather history data
@ -260,8 +263,14 @@ function createChartHook(configKey: string): Hook {
return { return {
mounted() { mounted() {
const self = this as ChartHookContext; const self = this as ChartHookContext;
self.isDestroyed = false;
self.chartLoadRetryTimer = null;
self.chartLoadRetryCount = 0;
registerChartInstance(self.el, self); registerChartInstance(self.el, self);
self.renderChart = () => { self.renderChart = () => {
if (self.isDestroyed || !self.el.isConnected) {
return;
}
if (self.chart) self.chart.destroy(); if (self.chart) self.chart.destroy();
const data: WeatherHistoryDatum[] = parseWeatherHistory( const data: WeatherHistoryDatum[] = parseWeatherHistory(
@ -360,11 +369,24 @@ function createChartHook(configKey: string): Hook {
// Check if Chart.js is loaded // Check if Chart.js is loaded
if (!window.Chart) { if (!window.Chart) {
if ((self.chartLoadRetryCount || 0) >= 20) {
console.error("Chart.js failed to load for weather chart");
return;
}
console.warn("Chart.js not loaded yet, retrying..."); console.warn("Chart.js not loaded yet, retrying...");
setTimeout(() => self.renderChart(), 100); self.chartLoadRetryCount = (self.chartLoadRetryCount || 0) + 1;
if (self.chartLoadRetryTimer) {
clearTimeout(self.chartLoadRetryTimer);
}
self.chartLoadRetryTimer = setTimeout(() => {
self.chartLoadRetryTimer = null;
self.renderChart();
}, 100);
return; return;
} }
self.chartLoadRetryCount = 0;
self.chart = new window.Chart(canvas, chartConfig); self.chart = new window.Chart(canvas, chartConfig);
}; };
@ -381,11 +403,19 @@ function createChartHook(configKey: string): Hook {
}, },
updated() { updated() {
(this as ChartHookContext).renderChart(); const self = this as ChartHookContext;
if (!self.isDestroyed) {
self.renderChart();
}
}, },
destroyed() { destroyed() {
const self = this as ChartHookContext; const self = this as ChartHookContext;
self.isDestroyed = true;
if (self.chartLoadRetryTimer) {
clearTimeout(self.chartLoadRetryTimer);
self.chartLoadRetryTimer = null;
}
if (self.themeChangeHandler) { if (self.themeChangeHandler) {
window.removeEventListener("themeChanged", self.themeChangeHandler); window.removeEventListener("themeChanged", self.themeChangeHandler);
} }

View file

@ -9,6 +9,7 @@ interface InfoMapContext {
lastSymbolHtml: string | null; lastSymbolHtml: string | null;
initializing: boolean; initializing: boolean;
resizeTimer: ReturnType<typeof setTimeout> | null; resizeTimer: ReturnType<typeof setTimeout> | null;
destroyed?: boolean;
} }
export const InfoMap = { export const InfoMap = {
@ -18,6 +19,7 @@ export const InfoMap = {
this.lastSymbolHtml = null; this.lastSymbolHtml = null;
this.initializing = false; this.initializing = false;
this.resizeTimer = null; this.resizeTimer = null;
this.destroyed = false;
initializeMap.call(this); initializeMap.call(this);
}, },
@ -64,6 +66,7 @@ export const InfoMap = {
}, },
destroyed(this: InfoMapContext) { destroyed(this: InfoMapContext) {
this.destroyed = true;
if (this.resizeTimer) { if (this.resizeTimer) {
clearTimeout(this.resizeTimer); clearTimeout(this.resizeTimer);
this.resizeTimer = null; this.resizeTimer = null;
@ -79,7 +82,7 @@ export const InfoMap = {
}; };
function initializeMap(this: InfoMapContext) { function initializeMap(this: InfoMapContext) {
if (this.initializing) return; if (this.initializing || this.destroyed) return;
if (typeof L === "undefined") { if (typeof L === "undefined") {
console.error("Leaflet not loaded for InfoMap"); console.error("Leaflet not loaded for InfoMap");
@ -135,7 +138,7 @@ function initializeMap(this: InfoMapContext) {
const map = this.map; const map = this.map;
this.resizeTimer = setTimeout(() => { this.resizeTimer = setTimeout(() => {
if (map) { if (!this.destroyed && this.el.isConnected && map && this.map === map) {
map.invalidateSize(); map.invalidateSize();
} }
this.resizeTimer = null; this.resizeTimer = null;