fix: code review refinements
- RegexCache: change ETS table from :public to :protected for consistency with other tables, preventing uncontrolled writes bypassing GenServer - MobileChannel: ensure_float now returns nil instead of the original unparsed string on parse failure, so validate_bounds properly rejects it - map.ts: remove dead commented-out localStorage code - map.ts: use instanceof HTMLAnchorElement instead of unsafe type cast in popup navigation handler - TrailManager: add destroyed flag to prevent stale RAF and debounce callbacks from firing after destroy, and clean up hover timer on destroy https://claude.ai/code/session_01Ps7Zq3wiBur1RtRKN7JM6X
This commit is contained in:
parent
36a86114a7
commit
a8051ce247
4 changed files with 13 additions and 31 deletions
|
|
@ -60,6 +60,7 @@ export class TrailManager {
|
||||||
private onTrailHoverEnd?: () => void;
|
private onTrailHoverEnd?: () => void;
|
||||||
private trailHoverDebounceTimer?: ReturnType<typeof setTimeout>;
|
private trailHoverDebounceTimer?: ReturnType<typeof setTimeout>;
|
||||||
private lastHoveredPath?: string;
|
private lastHoveredPath?: string;
|
||||||
|
private isDestroyed: boolean = false;
|
||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
trailLayer: LayerGroup,
|
trailLayer: LayerGroup,
|
||||||
|
|
@ -227,6 +228,7 @@ export class TrailManager {
|
||||||
|
|
||||||
private flushPendingTrailUpdates() {
|
private flushPendingTrailUpdates() {
|
||||||
this.trailUpdateRafId = null;
|
this.trailUpdateRafId = null;
|
||||||
|
if (this.isDestroyed) return;
|
||||||
for (const [callsign, isHistorical] of this.pendingTrailUpdates) {
|
for (const [callsign, isHistorical] of this.pendingTrailUpdates) {
|
||||||
const trailState = this.trails.get(callsign);
|
const trailState = this.trails.get(callsign);
|
||||||
if (trailState) {
|
if (trailState) {
|
||||||
|
|
@ -482,7 +484,7 @@ export class TrailManager {
|
||||||
if (path !== this.lastHoveredPath) {
|
if (path !== this.lastHoveredPath) {
|
||||||
this.lastHoveredPath = path;
|
this.lastHoveredPath = path;
|
||||||
this.trailHoverDebounceTimer = setTimeout(() => {
|
this.trailHoverDebounceTimer = setTimeout(() => {
|
||||||
hoverCb(mouseLat, mouseLng, path);
|
if (!this.isDestroyed) hoverCb(mouseLat, mouseLng, path);
|
||||||
}, 50);
|
}, 50);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
@ -557,10 +559,15 @@ export class TrailManager {
|
||||||
}
|
}
|
||||||
|
|
||||||
destroy() {
|
destroy() {
|
||||||
|
this.isDestroyed = true;
|
||||||
if (this.trailUpdateRafId !== null) {
|
if (this.trailUpdateRafId !== null) {
|
||||||
cancelAnimationFrame(this.trailUpdateRafId);
|
cancelAnimationFrame(this.trailUpdateRafId);
|
||||||
this.trailUpdateRafId = null;
|
this.trailUpdateRafId = null;
|
||||||
}
|
}
|
||||||
|
if (this.trailHoverDebounceTimer) {
|
||||||
|
clearTimeout(this.trailHoverDebounceTimer);
|
||||||
|
this.trailHoverDebounceTimer = undefined;
|
||||||
|
}
|
||||||
this.pendingTrailUpdates.clear();
|
this.pendingTrailUpdates.clear();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -142,31 +142,6 @@ let MapAPRSMap = {
|
||||||
initialZoom = 5;
|
initialZoom = 5;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Only use localStorage if no URL params are present
|
|
||||||
// Temporarily disabled - using server-side geolocation instead
|
|
||||||
/*
|
|
||||||
if (!useUrlParams) {
|
|
||||||
try {
|
|
||||||
const saved = localStorage.getItem("aprs_map_state");
|
|
||||||
if (saved) {
|
|
||||||
const { lat, lng, zoom } = JSON.parse(saved);
|
|
||||||
if (
|
|
||||||
typeof lat === "number" &&
|
|
||||||
typeof lng === "number" &&
|
|
||||||
typeof zoom === "number" &&
|
|
||||||
self.isValidCoordinate(lat, lng) &&
|
|
||||||
zoom >= 1 &&
|
|
||||||
zoom <= 20
|
|
||||||
) {
|
|
||||||
initialCenter = { lat, lng };
|
|
||||||
initialZoom = zoom;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} catch (e) {
|
|
||||||
}
|
|
||||||
}
|
|
||||||
*/
|
|
||||||
|
|
||||||
self.initializeMap(initialCenter, initialZoom);
|
self.initializeMap(initialCenter, initialZoom);
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|
@ -841,9 +816,9 @@ let MapAPRSMap = {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if clicked element or its parent is a LiveView navigation link
|
// Check if clicked element or its parent is a LiveView navigation link
|
||||||
const navLink = target.closest(".aprs-lv-link") as HTMLAnchorElement;
|
const navLink = target.closest(".aprs-lv-link");
|
||||||
|
|
||||||
if (navLink && navLink.href) {
|
if (navLink instanceof HTMLAnchorElement && navLink.href) {
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
e.stopPropagation();
|
e.stopPropagation();
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -18,7 +18,7 @@ defmodule Aprsme.RegexCache do
|
||||||
@impl true
|
@impl true
|
||||||
def init(_) do
|
def init(_) do
|
||||||
# Create ETS table for caching compiled regexes
|
# Create ETS table for caching compiled regexes
|
||||||
:ets.new(@table_name, [:set, :public, :named_table, read_concurrency: true])
|
:ets.new(@table_name, [:set, :protected, :named_table, read_concurrency: true])
|
||||||
{:ok, %{}}
|
{:ok, %{}}
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -302,11 +302,11 @@ defmodule AprsmeWeb.MobileChannel do
|
||||||
defp ensure_float(value) when is_binary(value) do
|
defp ensure_float(value) when is_binary(value) do
|
||||||
case Float.parse(String.trim(value)) do
|
case Float.parse(String.trim(value)) do
|
||||||
{float, ""} -> float
|
{float, ""} -> float
|
||||||
_ -> value
|
_ -> nil
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp ensure_float(value), do: value
|
defp ensure_float(_value), do: nil
|
||||||
|
|
||||||
defp ensure_integer(value, _default) when is_integer(value), do: value
|
defp ensure_integer(value, _default) when is_integer(value), do: value
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue