Skip to content

Re-implementation of PR #533 #541

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.

Sign up for GitHub

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

Open
ngoiyaeric wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Re-implementation of PR #533 #541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
429 changes: 145 additions & 284 deletions app/actions.tsx
View file
Open in desktop

Large diffs are not rendered by default.

177 changes: 177 additions & 0 deletions app/api/elevation/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
import { NextRequest, NextResponse } from 'next/server';
import * as turf from '@turf/turf';

/**
* GET /api/elevation
*
* Fetches elevation data for a grid of points within the given bounds.
* Uses Mapbox Terrain vector tileset to get elevation values.
*/
export async function GET(req: NextRequest) {
try {
const { searchParams } = new URL(req.url);
const boundsParam = searchParams.get('bounds');
const gridSizeParam = searchParams.get('gridSize');
const geometryParam = searchParams.get('geometry');

if (!boundsParam) {
return NextResponse.json(
{ error: 'Missing required parameter: bounds' },
{ status: 400 }
);
}

let bounds;
try {
bounds = JSON.parse(boundsParam);
} catch (e) {
return NextResponse.json({ error: 'Invalid bounds parameter' }, { status: 400 });
}

const gridSize = gridSizeParam ? parseInt(gridSizeParam) : 20;
const geometry = geometryParam ? JSON.parse(geometryParam) : null;
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

geometryParam JSON parse is unguarded — invalid JSON returns 500 instead of 400.

boundsParam has a dedicated try/catch returning a 400, but geometryParam is parsed inline on line 32 without a guard. An invalid JSON geometry string falls through to the outer catch and returns a generic 500 error.

🐛 Proposed fix 
-    const geometry = geometryParam ? JSON.parse(geometryParam) : null;
+    let geometry = null;
+    if (geometryParam) {
+      try {
+        geometry = JSON.parse(geometryParam);
+      } catch {
+        return NextResponse.json({ error: 'Invalid geometry parameter' }, { status: 400 });
+      }
+    }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` at line 32, The code parses geometryParam inline
(const geometry = geometryParam ? JSON.parse(geometryParam) : null;) which will
throw on invalid JSON and bubble to the outer catch returning 500; wrap the
JSON.parse for geometryParam in the same guarded try/catch used for boundsParam
so that invalid geometry JSON returns a 400 with a clear error message. Locate
geometryParam/geometry in the route handler (route.ts) and change the assignment
to parse inside a try block, set geometry to null or the parsed object on
success, and on JSON parse failure call the same bad request response flow used
for boundsParam.


const [west, south, east, north] = bounds;
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

bounds array length is not validated after parsing.

const [west, south, east, north] = bounds silently assigns undefined to east and north if the parsed array has fewer than 4 elements. All derived coordinates become NaN, every Mapbox fetch fails, and a silent empty success is returned. A length check should be added alongside the type check.

🐛 Proposed fix 
     let bounds;
     try {
       bounds = JSON.parse(boundsParam);
     } catch (e) {
       return NextResponse.json({ error: 'Invalid bounds parameter' }, { status: 400 });
     }
+
+    if (!Array.isArray(bounds) || bounds.length !== 4 || bounds.some((v: unknown) => typeof v !== 'number')) {
+      return NextResponse.json({ error: 'bounds must be an array of 4 numbers [west, south, east, north]' }, { status: 400 });
+    }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` at line 34, The destructuring const [west, south,
east, north] = bounds can produce undefined values if bounds.length < 4; update
the route handler to validate bounds is an array of length 4 (in addition to
current type checks) before destructuring, and return a 400/appropriate error
when the length is invalid; reference the bounds variable and the destructuring
expression (const [west, south, east, north] = bounds) so you add the length
check immediately prior to that line and avoid proceeding to coordinate parsing
and Mapbox fetches when the check fails.


const points: Array<{ lng: number; lat: number; elevation: number | null }> = [];
const lonStep = (east - west) / gridSize;
const latStep = (north - south) / gridSize;

const polygon = geometry ? turf.polygon(geometry.coordinates) : null;

Comment on lines +31 to +41
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

geometryParam is parsed with JSON.parse without a try/catch, while boundsParam is guarded. A single malformed geometry query parameter will throw and return a 500 instead of a 400.

Also, turf.polygon(geometry.coordinates) assumes the incoming geometry always matches what turf.polygon expects (array of linear rings). If geometry is a GeoJSON Polygon object, you should pass coordinates but validate geometry.type === 'Polygon' first to avoid hard failures.

Suggestion

Wrap geometry parsing in a try/catch and validate the geometry type before constructing a turf polygon:

let geometry: any = null
if (geometryParam) {
  try { geometry = JSON.parse(geometryParam) } catch { return NextResponse.json({ error: 'Invalid geometry parameter' }, { status: 400 }) }
  if (geometry?.type !== 'Polygon' || !Array.isArray(geometry.coordinates)) {
    return NextResponse.json({ error: 'geometry must be a GeoJSON Polygon' }, { status: 400 })
  }
}
const polygon = geometry ? turf.polygon(geometry.coordinates) : null

Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing this validation.

for (let i = 0; i <= gridSize; i++) {
for (let j = 0; j <= gridSize; j++) {
const lng = west + (lonStep * i);
const lat = south + (latStep * j);

if (polygon) {
const point = turf.point([lng, lat]);
if (!turf.booleanPointInPolygon(point, polygon)) {
continue;
}
}

points.push({ lng, lat, elevation: null });
}
}

const token = process.env.MAPBOX_ACCESS_TOKEN || process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;

// Fetch elevation data using Mapbox Terrain vector tiles (v2)
// This tileset contains contour lines with 'ele' property
const elevationPromises = points.map(async (point) => {
try {
const response = await fetch(
`https://api.mapbox.com/v4/mapbox.mapbox-terrain-v2/tilequery/${point.lng},${point.lat}.json?access_token=${token}`
);

if (response.ok) {
const data = await response.json();
if (data.features && data.features.length > 0) {
// Find the feature with the highest elevation in this point's vicinity
// or just take the first one if it has 'ele'
const elevations = data.features
.map((f: any) => f.properties.ele)
.filter((e: any) => e !== undefined);

if (elevations.length > 0) {
point.elevation = Math.max(...elevations);
}
}
}
} catch (error) {
console.error(`Error fetching elevation for ${point.lng},${point.lat}:`, error);
}
return point;
});
Comment on lines +58 to +86
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing Mapbox token guard — all 441 requests fail silently, returning a false-success empty response.

If both MAPBOX_ACCESS_TOKEN and NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN are unset, token is undefined and every fetch URL contains ?access_token=undefined. Each per-point fetch fails with 401, the error is swallowed in the per-point catch, validPoints ends up empty, and the handler returns { success: true, points: [], statistics: { min: 0, ... } } — indistinguishable from a valid empty response. Add an early guard:

🐛 Proposed fix 
     const token = process.env.MAPBOX_ACCESS_TOKEN || process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;
+    if (!token) {
+      return NextResponse.json(
+        { error: 'Mapbox access token is not configured' },
+        { status: 500 }
+      );
+    }
 
     // Fetch elevation data using Mapbox Terrain vector tiles (v2)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const token = process.env.MAPBOX_ACCESS_TOKEN || process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;
// Fetch elevation data using Mapbox Terrain vector tiles (v2)
// This tileset contains contour lines with 'ele' property
const elevationPromises = points.map(async (point) => {
try {
const response = await fetch(
`https://api.mapbox.com/v4/mapbox.mapbox-terrain-v2/tilequery/${point.lng},${point.lat}.json?access_token=${token}`
);
if (response.ok) {
const data = await response.json();
if (data.features && data.features.length > 0) {
// Find the feature with the highest elevation in this point's vicinity
// or just take the first one if it has 'ele'
const elevations = data.features
.map((f: any) => f.properties.ele)
.filter((e: any) => e !== undefined);
if (elevations.length > 0) {
point.elevation = Math.max(...elevations);
}
}
}
} catch (error) {
console.error(`Error fetching elevation for ${point.lng},${point.lat}:`, error);
}
return point;
});
const token = process.env.MAPBOX_ACCESS_TOKEN || process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;
if (!token) {
return NextResponse.json(
{ error: 'Mapbox access token is not configured' },
{ status: 500 }
);
}
// Fetch elevation data using Mapbox Terrain vector tiles (v2)
// This tileset contains contour lines with 'ele' property
const elevationPromises = points.map(async (point) => {
try {
const response = await fetch(
`https://api.mapbox.com/v4/mapbox.mapbox-terrain-v2/tilequery/${point.lng},${point.lat}.json?access_token=${token}`
);
if (response.ok) {
const data = await response.json();
if (data.features && data.features.length > 0) {
// Find the feature with the highest elevation in this point's vicinity
// or just take the first one if it has 'ele'
const elevations = data.features
.map((f: any) => f.properties.ele)
.filter((e: any) => e !== undefined);
if (elevations.length > 0) {
point.elevation = Math.max(...elevations);
}
}
}
} catch (error) {
console.error(`Error fetching elevation for ${point.lng},${point.lat}:`, error);
}
return point;
});
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` around lines 58 - 86, The code does not guard
against a missing Mapbox token (the token variable used in the fetch URL),
causing every request in elevationPromises (points.map(...)) to silently fail;
add an early check before creating elevationPromises to validate token (from
MAPBOX_ACCESS_TOKEN or NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN) and short-circuit the
handler with an explicit error/failed response (or throw) if absent so you don't
perform the per-point fetches and return a false-success payload; update the
handler entrypoint that consumes elevationPromises to rely on this guard and
ensure the error is logged and returned to the client instead of letting the
per-point catch blocks hide 401 failures.


const elevationData = await Promise.all(elevationPromises);
Comment on lines +62 to +88
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

441 concurrent Mapbox tilequery requests per API call — risks rate-limiting and serverless timeout.

With the default gridSize of 20, the loop produces (20+1)² = 441 points. Promise.all(elevationPromises) fires all 441 as simultaneous HTTP requests. Mapbox's tilequery API rate limit is ~600 requests/minute, meaning a single call from one user can exhaust ~73% of that budget. On Vercel's default 10-second function timeout, sequential DNS + TLS + HTTP for 441 calls is unreachable even in parallel given real-world latency variance.

Consider one of: reducing the default gridSize, batching requests with controlled concurrency (e.g., chunks of 20–50), or using Mapbox Terrain DEM raster tiles directly (a single tile covers many coordinates).

♻️ Example: controlled concurrency with batching 
// Process in batches of 50 to limit concurrent requests
const BATCH_SIZE = 50;
const results: typeof points = [];
for (let i = 0; i < points.length; i += BATCH_SIZE) {
  const batch = points.slice(i, i + BATCH_SIZE);
  const batchResults = await Promise.all(batch.map(async (point) => {
    // ... existing fetch logic
  }));
  results.push(...batchResults);
}
const elevationData = results;
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` around lines 62 - 88, The current code creates
elevationPromises for every point and calls Promise.all(elevationPromises),
which issues up to 441 concurrent Mapbox tilequery requests; replace this with
controlled concurrency/batching to avoid rate limits and timeouts by processing
points in chunks (e.g., BATCH_SIZE = 50) or using a concurrency limiter (like
p-limit) so only a limited number of fetches run concurrently; specifically,
change the logic around points, elevationPromises and elevationData to iterate
over points in slices (points.slice(i, i + BATCH_SIZE)) or wrap the fetch logic
in a limited-run queue, await each batch (or limited-run promises) before
continuing, and accumulate results into elevationData instead of firing all
requests at once.

const validPoints = elevationData.filter(p => p.elevation !== null);

Comment on lines +58 to +90
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The elevation API performs a Mapbox Tilequery request for every grid point concurrently via Promise.all(points.map(...)). With the default (gridSize+1)^2 points, gridSize=20 yields 441 outbound requests per API call, which is very likely to trigger rate limits and will also be slow/expensive. Additionally, there’s no guard if MAPBOX_ACCESS_TOKEN is missing—requests will still be attempted with access_token=null/undefined.

This endpoint will be unstable under real traffic and can cause cascading latency issues.

Suggestion

Add (1) a required-token check, and (2) concurrency limiting + sane caps:

  • Reject or return { success: false } with 500/503 if token is missing.
  • Cap gridSize (e.g. <= 30) and/or total points.
  • Limit concurrency with a pool (e.g. p-limit) and consider caching results by rounded lat/lng.

Sketch:

if (!token) return NextResponse.json({ error: 'Mapbox token missing' }, { status: 500 })
const gridSize = Math.min(parsedGridSize, 30)
const limit = pLimit(10)
const elevationData = await Promise.all(points.map(p => limit(() => fetchElevation(p))))

Reply with "@CharlieHelps yes please" if you'd like me to add a commit adding token validation + concurrency limiting and a gridSize cap.

if (validPoints.length === 0) {
return NextResponse.json({
success: true,
points: [],
statistics: { min: 0, max: 0, average: 0, count: 0 },
bounds,
gridSize,
});
}

const elevations = validPoints.map(p => p.elevation as number);
const minElevation = Math.min(...elevations);
const maxElevation = Math.max(...elevations);
const avgElevation = elevations.reduce((sum, e) => sum + e, 0) / elevations.length;

return NextResponse.json({
success: true,
points: validPoints,
statistics: {
min: minElevation,
max: maxElevation,
average: Math.round(avgElevation * 10) / 10,
count: validPoints.length,
},
bounds,
gridSize,
});

} catch (error) {
console.error('Error fetching elevation data:', error);
return NextResponse.json(
{
error: 'Failed to fetch elevation data',
details: error instanceof Error ? error.message : 'Unknown error'
},
{ status: 500 }
);
}
}

export async function POST(req: NextRequest) {
try {
const body = await req.json();
const { features, gridSize = 20 } = body;

if (!features || !Array.isArray(features)) {
return NextResponse.json(
{ error: 'Missing or invalid features array' },
{ status: 400 }
);
}

const results = await Promise.all(
features.map(async (feature: any) => {
if (feature.geometry.type !== 'Polygon') {
return null;
}
Comment on lines +144 to +147
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

feature.geometry is not null-checked in the POST handler — will throw TypeError on malformed input.

feature.geometry.type !== 'Polygon' throws TypeError: Cannot read properties of null if any feature arrives without a geometry field. This crashes the Promise.all and surfaces as a 500 with the raw error message.

🐛 Proposed fix 
-        if (feature.geometry.type !== 'Polygon') {
+        if (!feature.geometry || feature.geometry.type !== 'Polygon') {
           return null;
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
features.map(async (feature: any) => {
if (feature.geometry.type !== 'Polygon') {
return null;
}
features.map(async (feature: any) => {
if (!feature.geometry || feature.geometry.type !== 'Polygon') {
return null;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` around lines 144 - 147, The POST handler's
features.map callback accesses feature.geometry.type without null-checking,
which throws when geometry is missing; update the mapping in the POST handler
(the async callback passed to features.map) to defensively check for feature and
feature.geometry (e.g., use optional chaining or explicit null checks) before
reading .type and skip/return null for malformed features so Promise.all won't
reject on a TypeError.


const bbox = turf.bbox(feature);
const params = new URLSearchParams({
bounds: JSON.stringify(bbox),
gridSize: gridSize.toString(),
geometry: JSON.stringify(feature.geometry),
});

const url = new URL(`/api/elevation?${params}`, req.url);
const response = await GET(new NextRequest(url));
return await response.json();
})
Comment on lines +131 to +159
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In POST, the handler calls GET(new NextRequest(url)). This is an unusual pattern and can be problematic because it bypasses normal Next.js request handling/middleware and can lose request context. It also re-parses URL params and repeats validation paths.

This is better implemented by extracting the core elevation sampling logic into a shared function and calling it from both GET and POST.

Suggestion

Refactor to a shared internal function like computeElevation({ bounds, gridSize, geometry }) and call it from both handlers. This avoids constructing synthetic requests and keeps behavior consistent.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit performing this refactor.

);

return NextResponse.json({
success: true,
results: results.filter(r => r !== null),
});

} catch (error) {
console.error('Error in batch elevation query:', error);
return NextResponse.json(
{
error: 'Failed to process batch elevation query',
details: error instanceof Error ? error.message : 'Unknown error'
},
{ status: 500 }
);
}
}
23 changes: 2 additions & 21 deletions app/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Metadata, Viewport } from 'next'
import Script from 'next/script'
import { Inter as FontSans, Poppins } from 'next/font/google'
import './globals.css'
import 'katex/dist/katex.min.css';
Expand Down Expand Up @@ -66,27 +67,7 @@ export default function RootLayout({
return (
<html lang="en" suppressHydrationWarning>
<head>
<script
dangerouslySetInnerHTML={{
__html: `
(function() {
const htmxEvents = [
'sseError', 'sseOpen', 'swapError', 'targetError', 'timeout',
'validation:validate', 'validation:failed', 'validation:halted',
'xhr:abort', 'xhr:loadend', 'xhr:loadstart'
];
htmxEvents.forEach(event => {
const funcName = 'func ' + event;
if (typeof window[funcName] === 'undefined') {
window[funcName] = function() {
console.warn('HTMX event handler "' + funcName + '" was called but not defined. Providing safety fallback.');
};
}
});
})();
`,
}}
/>
<Script src="/htmx-fallback.js" strategy="beforeInteractive" />
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Next.js 15 Script component beforeInteractive App Router placement head JSX

💡 Result:

In Next.js 15 (App Router), a <Script strategy="beforeInteractive" /> must be declared in the Root Layout (app/layout.tsx|js|jsx). If you put it in a page, template, or nested layout, you’ll get the “No Before Interactive Script Outside Document” error. [1][2]

You do not need to place it inside <head> JSX. For beforeInteractive, Next.js will always inject it into the document <head> in the server-rendered HTML regardless of where it appears in your Root Layout’s JSX. [1]

Example (recommended pattern):

// app/layout.tsx
import Script from "next/script";

export default function RootLayout({ children }: { children: React.ReactNode }) {
  return (
    <html lang="en">
      <body>
        {children}

        <Script
          src="https://example.com/script.js"
          strategy="beforeInteractive"
        />
      </body>
    </html>
  );
}

Sources: Next.js Script docs [1] and the corresponding error docs [2].

🌐 Web query:

Next.js App Router Script component canonical pattern placement best practice

💡 Result:

Canonical pattern (App Router) for next/script placement

  • Default best practice: colocate scripts with the smallest scope that needs them (page-level or segment layout.tsx), to avoid site-wide performance cost. Next.js explicitly recommends only including third‑party scripts in specific pages/layouts when possible. [1]
  • If a script is needed across a route segment, put it in that segment’s app/<segment>/layout.tsx; it will load when any route under that layout is visited, and Next.js ensures it only loads once. [1]
  • If a script is truly needed site-wide, put it in the root layout app/layout.tsx. [1]

Strategy-specific placement rule (important)

  • strategy="beforeInteractive" must be placed in the root layout (app/layout.tsx) (intended for critical, site-wide scripts like consent managers / bot detection). It will be injected into <head> regardless of where you place the component. [2]
  • afterInteractive (default) and lazyOnload can be placed in any page or layout and will only load when that page/layout is visited. [2]
  • worker is still experimental and (as of Feb 11, 2026) does not work with the App Router. [1]

Typical layouts

// app/(marketing)/layout.tsx  (segment-scoped)
import Script from "next/script";

export default function MarketingLayout({ children }: { children: React.ReactNode }) {
  return (
    <>
      {children}
      <Script src="https://example.com/marketing-analytics.js" strategy="afterInteractive" />
    </>
  );
}
// app/layout.tsx  (site-wide + beforeInteractive)
import Script from "next/script";

export default function RootLayout({ children }: { children: React.ReactNode }) {
  return (
    <html lang="en">
      <body>
        {children}
        <Script src="https://example.com/consent.js" strategy="beforeInteractive" />
      </body>
    </html>
  );
}

Sources: Next.js “Scripts” guide (updated Feb 11, 2026) [1], Next.js next/script API reference (placement + strategy rules) [2].

🌐 Web query:

Next.js beforeInteractive vs afterInteractive strategy when to use

💡 Result:

beforeInteractive — use only when the script must run before hydration

Use this when the page/app could break or behave incorrectly unless the script is available before any Next.js code runs (i.e., before hydration). Next.js describes it as loading “before any Next.js code and before any page hydration occurs.” [1]

Typical use cases:

  • Critical polyfills required for your app code to run
  • “Must-run-first” bootstrapping like theme initialization to avoid FOUC, or privacy/consent gating that must block other scripts until configured

Trade-off: it can delay the page from becoming interactive, so it should be rare. [2]

afterInteractive (default) — use for almost everything else

Use this for scripts that should load early, but don’t need to block hydration. Next.js describes it as loading early “but after some hydration on the page occurs.” [1]

Typical use cases:

  • Analytics / tag managers
  • Customer support chat widgets
  • A/B testing, heatmaps, user tracking that doesn’t need to run before the app starts

Next.js learning material also recommends deferring most third-party scripts until after the page is interactive (or even later) to avoid hurting performance (e.g., LCP). [3]

Rule of thumb: default to afterInteractive; choose beforeInteractive only if the script must execute before React/Next hydrates to prevent broken UI/logic or to enforce “run-before-anything” requirements. [1][2][3]

Sources: Next.js Scripts guide [1], Vercel conformance note on beforeInteractive impact [2], Next.js “Optimizing Third-Party Scripts” learn page [3].

Use strategy="afterInteractive" for HTMX fallback stubs and follow the canonical Script component pattern.

The beforeInteractive strategy is intended only for scripts that must run before page hydration. Since these are HTMX fallback stubs that fire on HTMX events (which occur after the page loads), afterInteractive is more appropriate and won't delay page interactivity.

Additionally, while beforeInteractive scripts are always injected into the document <head> regardless of placement, the canonical Next.js pattern (per official docs) places the Script component as a sibling to <body>, not inside the manually written <head> JSX element. Placing it inside the manual <head> is non-standard.

🛠️ Suggested fix 
-      <head>
-        <Script src="/htmx-fallback.js" strategy="beforeInteractive" />
-      </head>

And add the Script as a sibling to <body>:

       </body>
+      <Script src="/htmx-fallback.js" strategy="afterInteractive" />
     </html>
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/layout.tsx` at line 70, The Script instance used for the HTMX fallback
stub (the <Script ... /> in app layout) should use strategy="afterInteractive"
and be moved out of the manual <head> JSX into a sibling of the <body> element
per Next.js canonical pattern; update the Script component invocation that
references "/htmx-fallback.js" to change strategy to "afterInteractive" and
relocate that JSX so it is rendered alongside (not inside) the <body> element.

</head>
<body
className={cn(
Expand Down
6 changes: 3 additions & 3 deletions components/header-search-button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Button } from '@/components/ui/button'
import { Search } from 'lucide-react'
import { useMap } from './map/map-context'
import { useActions, useUIState } from 'ai/rsc'
import { AI } from '@/app/actions'

import { nanoid } from '@/lib/utils'
import { UserMessage } from './user-message'
import { toast } from 'sonner'
Expand All @@ -24,8 +24,8 @@ export function HeaderSearchButton() {
const { mapProvider } = useSettingsStore()
const { mapData } = useMapData()
// Cast the actions to our defined interface to avoid build errors
const actions = useActions<typeof AI>() as unknown as HeaderActions
const [, setMessages] = useUIState<typeof AI>()
const actions = useActions<any>() as unknown as HeaderActions
const [, setMessages] = useUIState<any>()
const [isAnalyzing, setIsAnalyzing] = useState(false)
const [desktopPortal, setDesktopPortal] = useState<HTMLElement | null>(null)
const [mobilePortal, setMobilePortal] = useState<HTMLElement | null>(null)
Expand Down
143 changes: 143 additions & 0 deletions components/map/elevation-heatmap-layer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
'use client'

import { useEffect, useState } from 'react'
import { useMap } from './map-context'

interface ElevationPoint {
lng: number;
lat: number;
elevation: number;
}

interface ElevationHeatmapLayerProps {
id: string;
points: ElevationPoint[];
statistics?: {
min: number;
max: number;
average: number;
count: number;
};
}

export function ElevationHeatmapLayer({ id, points, statistics }: ElevationHeatmapLayerProps) {
const { map } = useMap()
const [mapboxgl, setMapboxgl] = useState<any>(null)
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

useState<any>(null) loses type safety for the dynamically imported mapboxgl module.

Consider using the proper type:

♻️ Suggested refinement 
-  const [mapboxgl, setMapboxgl] = useState<any>(null)
+  const [mapboxgl, setMapboxgl] = useState<typeof import('mapbox-gl') | null>(null)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const [mapboxgl, setMapboxgl] = useState<any>(null)
const [mapboxgl, setMapboxgl] = useState<typeof import('mapbox-gl') | null>(null)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/map/elevation-heatmap-layer.tsx` at line 25, The state for the
dynamically imported mapbox module loses type safety; update the useState
declaration for the mapboxgl variable to a proper Mapbox GL type (e.g., use a
typeof import('mapbox-gl') or an explicit MapboxGL type) so
setMapboxgl/getMapboxgl are strongly typed. Add an import type for the mapbox-gl
module (import type mapboxgl from 'mapbox-gl' or use the import('mapbox-gl')
type inline) and change const [mapboxgl, setMapboxgl] = useState<any>(null) to
useState<typeof import('mapbox-gl') | null>(null) (or useState<mapboxgl |
null>(null)) so all usages of mapboxgl and setMapboxgl have correct types.


useEffect(() => {
import('mapbox-gl').then(mod => {
setMapboxgl(mod.default)
})
}, [])

useEffect(() => {
if (!map || !points || points.length === 0 || !mapboxgl) return

const sourceId = `elevation-heatmap-source-${id}`
const heatmapLayerId = `elevation-heatmap-layer-${id}`
const pointsLayerId = `elevation-points-layer-${id}`

const geojson: any = {
type: 'FeatureCollection',
features: points.map(point => ({
type: 'Feature',
geometry: {
type: 'Point',
coordinates: [point.lng, point.lat]
},
properties: {
elevation: point.elevation,
intensity: statistics && statistics.max !== statistics.min
? (point.elevation - statistics.min) / (statistics.max - statistics.min)
: 0.5
}
}))
}

const onMapLoad = () => {
if (!map.getSource(sourceId)) {
map.addSource(sourceId, {
type: 'geojson',
data: geojson
})

map.addLayer({
id: heatmapLayerId,
type: 'heatmap',
source: sourceId,
paint: {
'heatmap-weight': ['interpolate', ['linear'], ['get', 'intensity'], 0, 0, 1, 1],
'heatmap-intensity': ['interpolate', ['linear'], ['zoom'], 0, 1, 15, 3],
'heatmap-color': [
'interpolate',
['linear'],
['heatmap-density'],
0, 'rgba(33,102,172,0)',
0.2, 'rgb(103,169,207)',
0.4, 'rgb(209,229,240)',
0.6, 'rgb(253,219,199)',
0.8, 'rgb(239,138,98)',
1, 'rgb(178,24,43)'
],
'heatmap-radius': ['interpolate', ['linear'], ['zoom'], 0, 2, 15, 20],
'heatmap-opacity': ['interpolate', ['linear'], ['zoom'], 7, 0.7, 15, 0.5]
}
})

map.addLayer({
id: pointsLayerId,
type: 'circle',
source: sourceId,
minzoom: 14,
paint: {
'circle-radius': ['interpolate', ['linear'], ['zoom'], 14, 3, 22, 8],
'circle-color': [
'interpolate',
['linear'],
['get', 'intensity'],
0, '#2166ac',
0.25, '#67a9cf',
0.5, '#f7f7f7',
0.75, '#ef8a62',
1, '#b2182b'
],
'circle-stroke-color': 'white',
'circle-stroke-width': 1,
'circle-opacity': ['interpolate', ['linear'], ['zoom'], 14, 0, 15, 0.8]
}
})

const clickHandler = (e: any) => {
if (!e.features || e.features.length === 0) return
const elevation = e.features[0].properties?.elevation
if (elevation !== undefined) {
new mapboxgl.Popup()
.setLngLat(e.lngLat)
.setHTML(`<strong>Elevation:</strong> ${elevation}m`)
.addTo(map)
}
}

map.on('click', pointsLayerId, clickHandler)
map.on('mouseenter', pointsLayerId, () => { map.getCanvas().style.cursor = 'pointer' })
map.on('mouseleave', pointsLayerId, () => { map.getCanvas().style.cursor = '' })
Comment on lines +110 to +123
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Map event listeners are not removed in cleanup — mouseenter/mouseleave use anonymous functions that can never be unregistered.

clickHandler is a named reference but is never passed to map.off(). The mouseenter and mouseleave handlers are anonymous arrow functions with no stored reference, so they cannot be removed at all. After unmount, the cursor handler remains active and can leave the cursor stuck as 'pointer'.

🐛 Proposed fix 
-        map.on('click', pointsLayerId, clickHandler)
-        map.on('mouseenter', pointsLayerId, () => { map.getCanvas().style.cursor = 'pointer' })
-        map.on('mouseleave', pointsLayerId, () => { map.getCanvas().style.cursor = '' })
+        const mouseEnterHandler = () => { map.getCanvas().style.cursor = 'pointer' }
+        const mouseLeaveHandler = () => { map.getCanvas().style.cursor = '' }
+        map.on('click', pointsLayerId, clickHandler)
+        map.on('mouseenter', pointsLayerId, mouseEnterHandler)
+        map.on('mouseleave', pointsLayerId, mouseLeaveHandler)

Then in the cleanup returned from the outer useEffect (alongside the source/layer removal):

+        map.off('click', pointsLayerId, clickHandler)
+        map.off('mouseenter', pointsLayerId, mouseEnterHandler)
+        map.off('mouseleave', pointsLayerId, mouseLeaveHandler)
         if (map.getLayer(pointsLayerId)) map.removeLayer(pointsLayerId)

Note: mouseEnterHandler and mouseLeaveHandler need to be hoisted to the outer useEffect scope (alongside clickHandler) so the cleanup closure can reference them.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const clickHandler = (e: any) => {
if (!e.features || e.features.length === 0) return
const elevation = e.features[0].properties?.elevation
if (elevation !== undefined) {
new mapboxgl.Popup()
.setLngLat(e.lngLat)
.setHTML(`<strong>Elevation:</strong> ${elevation}m`)
.addTo(map)
}
}
map.on('click', pointsLayerId, clickHandler)
map.on('mouseenter', pointsLayerId, () => { map.getCanvas().style.cursor = 'pointer' })
map.on('mouseleave', pointsLayerId, () => { map.getCanvas().style.cursor = '' })
const clickHandler = (e: any) => {
if (!e.features || e.features.length === 0) return
const elevation = e.features[0].properties?.elevation
if (elevation !== undefined) {
new mapboxgl.Popup()
.setLngLat(e.lngLat)
.setHTML(`<strong>Elevation:</strong> ${elevation}m`)
.addTo(map)
}
}
const mouseEnterHandler = () => { map.getCanvas().style.cursor = 'pointer' }
const mouseLeaveHandler = () => { map.getCanvas().style.cursor = '' }
map.on('click', pointsLayerId, clickHandler)
map.on('mouseenter', pointsLayerId, mouseEnterHandler)
map.on('mouseleave', pointsLayerId, mouseLeaveHandler)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/map/elevation-heatmap-layer.tsx` around lines 110 - 123, The map
event handlers are registered but never removed: hoist the handlers out of the
inline callbacks by defining clickHandler, mouseEnterHandler, and
mouseLeaveHandler in the same useEffect scope where you call
map.on(pointsLayerId, ...), replace the anonymous mouseenter/mouseleave arrow
functions with mouseEnterHandler and mouseLeaveHandler, and then in the
useEffect cleanup call map.off('click', pointsLayerId, clickHandler),
map.off('mouseenter', pointsLayerId, mouseEnterHandler), and
map.off('mouseleave', pointsLayerId, mouseLeaveHandler) so the handlers are
properly unregistered when the component unmounts (ensure you still remove the
popup behavior within clickHandler as before).

}
}

if (map.isStyleLoaded()) {
onMapLoad()
} else {
map.once('load', onMapLoad)
}

return () => {
if (map) {
if (map.getLayer(pointsLayerId)) map.removeLayer(pointsLayerId)
if (map.getLayer(heatmapLayerId)) map.removeLayer(heatmapLayerId)
if (map.getSource(sourceId)) map.removeSource(sourceId)
}
}
Comment on lines +57 to +139
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new ElevationHeatmapLayer registers event handlers (click, mouseenter, mouseleave) but the cleanup only removes layers/sources. If the layer is removed without also removing handlers, Mapbox can keep references around and leak handlers (or throw warnings).

Additionally, the onMapLoad function is registered via map.once('load', onMapLoad) but is never removed on cleanup if the component unmounts before load fires.

Suggestion

Store handlers and unregister them in cleanup, and cancel the pending load handler:

const clickHandler = (e: mapboxgl.MapLayerMouseEvent) => { ... }
const enterHandler = () => { map.getCanvas().style.cursor = 'pointer' }
const leaveHandler = () => { map.getCanvas().style.cursor = '' }

map.on('click', pointsLayerId, clickHandler)
map.on('mouseenter', pointsLayerId, enterHandler)
map.on('mouseleave', pointsLayerId, leaveHandler)

return () => {
  map.off('click', pointsLayerId, clickHandler)
  map.off('mouseenter', pointsLayerId, enterHandler)
  map.off('mouseleave', pointsLayerId, leaveHandler)
  map.off('load', onMapLoad)
  ...remove layers/sources...
}

Reply with "@CharlieHelps yes please" if you'd like me to add a commit addressing the handler cleanup properly.

Comment on lines +127 to +139
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

map.once('load', onMapLoad) is never cancelled in cleanup — orphaned layers will be added after unmount.

The cleanup function removes layers/sources but does not call map.off('load', onMapLoad). If the component unmounts (or dependencies change triggering re-run) while the map style is still loading, the pending onMapLoad callback fires after cleanup, adding the heatmap source and layers to the map permanently — with no subsequent cleanup to remove them.

🐛 Proposed fix 
+    let cancelled = false
+    const safeOnMapLoad = () => {
+      if (cancelled) return
+      onMapLoad()
+    }

     if (map.isStyleLoaded()) {
       onMapLoad()
     } else {
-      map.once('load', onMapLoad)
+      map.once('load', safeOnMapLoad)
     }

     return () => {
+      cancelled = true
+      map.off('load', safeOnMapLoad)
       if (map) {
         if (map.getLayer(pointsLayerId)) map.removeLayer(pointsLayerId)
         if (map.getLayer(heatmapLayerId)) map.removeLayer(heatmapLayerId)
         if (map.getSource(sourceId)) map.removeSource(sourceId)
       }
     }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/map/elevation-heatmap-layer.tsx` around lines 127 - 139, The
cleanup currently removes layers/sources but doesn't cancel a pending load
listener, so if map.once('load', onMapLoad) was registered and the component
unmounts the onMapLoad callback can still run and add orphaned layers; update
the cleanup to also call map.off('load', onMapLoad) (or remove the listener
registered) to ensure the onMapLoad handler is deregistered when the effect
cleans up, keeping references to the same onMapLoad function and leaving
existing removal of pointsLayerId, heatmapLayerId and sourceId intact.

}, [map, id, points, statistics, mapboxgl])

return null
}
Loading