Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion apps/geolibre-desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"i18next": "^26.3.6",
"jspdf": "^4.2.1",
"mapillary-js": "^4.1.2",
"maplibre-gl": "^5.24.0",
"maplibre-gl": "^6.1.0",
"maplibre-gl-3d-tiles": "^0.5.6",
"maplibre-gl-basemap-control": "^0.13.0",
"maplibre-gl-components": "^0.30.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useEffect, useRef } from "react";
import { useAppStore, type ProjectComment } from "@geolibre/core";
import type { MapController } from "@geolibre/map";
import maplibreGl from "maplibre-gl";
import * as maplibreGl from "maplibre-gl";

interface CommentMapOverlayProps {
mapControllerRef: React.RefObject<MapController | null>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import type { MapController } from "@geolibre/map";
import { v4 as uuidv4 } from "uuid";
import type { CollaborationApi } from "../../hooks/useCollaboration";
import type maplibreGl from "maplibre-gl";
import type * as maplibreGl from "maplibre-gl";

interface UseCommentToolOptions {
mapControllerRef: React.RefObject<MapController | null>;
Expand Down Expand Up @@ -90,7 +90,7 @@
setPendingComment(null);
setIsActive(false);
},
[pendingComment, collab, addComment, collaboration],

Check warning on line 93 in apps/geolibre-desktop/src/components/comments/useCommentTool.ts

View workflow job for this annotation

GitHub Actions / Build and test

React Hook useCallback has a missing dependency: 't'. Either include it or remove the dependency array
);

const cancelPendingComment = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useCallback, useEffect, useId, useMemo, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import maplibregl from "maplibre-gl";
import * as maplibregl from "maplibre-gl";
import type { MapController } from "@geolibre/map";
import {
getAttributeFormField,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import type maplibregl from "maplibre-gl";
import type * as maplibregl from "maplibre-gl";
import type { MapController } from "@geolibre/map";
import { DEFAULT_LAYER_STYLE, type GeoLibreLayer, useAppStore } from "@geolibre/core";
import {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useCallback, useEffect, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import type { TFunction } from "i18next";
import type { Feature, FeatureCollection } from "geojson";
import maplibregl from "maplibre-gl";
import * as maplibregl from "maplibre-gl";
import type { MapController } from "@geolibre/map";
import { useAppStore } from "@geolibre/core";
import {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
DropdownMenuSubTrigger,
DropdownMenuTrigger,
} from "@geolibre/ui";
import type maplibregl from "maplibre-gl";
import type * as maplibregl from "maplibre-gl";
import {
BookOpen,
Braces,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
useState,
} from "react";
import { useTranslation } from "react-i18next";
import maplibregl from "maplibre-gl";
import * as maplibregl from "maplibre-gl";
import type { MapController } from "@geolibre/map";
import { clamp } from "../../lib/clamp";
import { type ChartDomain, resolveChartDomain } from "../../lib/chart-domain";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useAppStore, type CollaborationPresence } from "@geolibre/core";
import maplibregl from "maplibre-gl";
import * as maplibregl from "maplibre-gl";
import { useEffect, useRef } from "react";
import type { RefObject } from "react";
import type { MapController } from "@geolibre/map";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
useState,
} from "react";
import { useTranslation } from "react-i18next";
import maplibregl from "maplibre-gl";
import * as maplibregl from "maplibre-gl";
import {
type GeocodeMatch,
geocodeForward,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { type RefObject, useCallback, useEffect, useMemo, useRef, useState } from "react";
import maplibregl from "maplibre-gl";
import * as maplibregl from "maplibre-gl";
import { useTranslation } from "react-i18next";
import type { StoryActiveSlideMode, StoryChapter, StoryMap } from "@geolibre/core";
import type { MapController } from "@geolibre/map";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from "react";
import { createPortal } from "react-dom";
import { useTranslation } from "react-i18next";
import maplibregl from "maplibre-gl";
import * as maplibregl from "maplibre-gl";
import {
useAppStore,
type StoryActiveSlideMode,
Expand Down
Binary file modified apps/geolibre-desktop/src/hooks/useCommandBridge.ts
Binary file not shown.
2 changes: 1 addition & 1 deletion apps/geolibre-desktop/src/hooks/useNotebookBridge.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useAppStore } from "@geolibre/core";
import type maplibregl from "maplibre-gl";
import type * as maplibregl from "maplibre-gl";
import { type RefObject, useEffect } from "react";
import type { MapController } from "@geolibre/map";
import { createScriptingHandlers } from "../lib/scripting/scriptingApi";
Expand Down
2 changes: 1 addition & 1 deletion apps/geolibre-desktop/src/lib/assistant/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from "@geolibre/core";
import type { MapController } from "@geolibre/map";
import type { InvokableTool, JSONValue } from "@strands-agents/sdk";
import maplibregl from "maplibre-gl";
import * as maplibregl from "maplibre-gl";
import { tool } from "@strands-agents/sdk";
import type { FeatureCollection } from "geojson";
import { z } from "zod";
Expand Down
8 changes: 4 additions & 4 deletions apps/geolibre-desktop/src/lib/kml-super-overlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ export function setKmlSuperOverlayResolver(resolver: KmlSuperOverlayResolver | n
async function ensureProtocol(): Promise<void> {
if (protocolRegistered) return;
// Keep MapLibre out of tauri-io's static module graph. This also lets the
// DOM-only file-loader tests import tauri-io in Node — where maplibre-gl
// resolves to its CJS build, so the named exports sit under `default`.
const maplibre = await import("maplibre-gl");
const addProtocol = maplibre.addProtocol ?? maplibre.default.addProtocol;
// DOM-only file-loader tests import tauri-io in Node without pulling the map
// in. MapLibre v6 is ESM-only with named exports and no default export, so
// this reads `addProtocol` straight off the namespace.
const { addProtocol } = await import("maplibre-gl");
addProtocol(PROTOCOL, handleTileRequest);
protocolRegistered = true;
}
Expand Down
22 changes: 22 additions & 0 deletions apps/geolibre-desktop/src/lib/maplibre-worker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { setWorkerUrl } from "maplibre-gl";
// Vite bundles the worker (it imports `./maplibre-gl-shared.mjs`) and hands back
// the emitted asset URL.
import maplibreWorkerUrl from "maplibre-gl/dist/maplibre-gl-worker.mjs?worker&url";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor/low-confidence nit: this hardcodes maplibre-gl/dist/maplibre-gl-worker.mjs as the worker entry path. The PR wisely added tests/maplibre-shim-parity.test.ts to guard the other MapLibre-v6 workaround (the import shim) against silently rotting when the dependency updates, but there's no equivalent guard here — if a future maplibre-gl patch renames/moves this dist file, the failure mode is exactly the one this PR just fixed (a silently-hanging worker request), and nothing in CI would catch it. Worth considering a cheap existence check (e.g. an e2e/smoke assertion that the worker actually loads, or at least a comment pointing at what to re-verify on maplibre-gl bumps) so a future Dependabot bump doesn't quietly reintroduce this bug.


/**
* Point MapLibre at its bundled worker.
*
* v6 ships the worker as a **separate file** and locates it at runtime with
* `new URL("./maplibre-gl-worker.mjs", import.meta.url)`. That is a computed
* string, not a static `new URL(…, import.meta.url)` literal, so no bundler can
* see it: the file is never emitted, and at runtime the URL resolves next to the
* hashed app chunk (`/assets/maplibre-gl-worker.mjs`), where nothing exists. In
* the web build the SPA fallback answers that request with `index.html`, so the
* failure is not even a 404 — the worker is handed HTML, and the request hangs
* rather than erroring.
*
* `setWorkerUrl` overrides that lookup with an asset the build actually emits.
* Must run before the first `Map` is constructed, so it is imported for effect
* from the app entry.
*/
setWorkerUrl(maplibreWorkerUrl);
2 changes: 1 addition & 1 deletion apps/geolibre-desktop/src/lib/print-layout-export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* export helpers rasterize {@link drawLayout} at print resolution.
*/
import { zipSync } from "fflate";
import jsPDF from "jspdf";
import { jsPDF } from "jspdf";
import { isFullViewportMapCanvas } from "./print-capture";
import { drawLayout, pageMm, pagePx, resolvePageSize, type LayoutOptions } from "./print-layout";
import type { PrintExtent } from "./print-extent";
Expand Down
2 changes: 1 addition & 1 deletion apps/geolibre-desktop/src/lib/rtl-text.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import maplibregl from "maplibre-gl";
import * as maplibregl from "maplibre-gl";
// Vite resolves this to a hashed, same-origin asset URL at build time, so the
// RTL plugin ships with the app (web, desktop, and the Jupyter embed) instead of
// being fetched from a CDN — keeps every build's CSP simple and works offline.
Expand Down
2 changes: 1 addition & 1 deletion apps/geolibre-desktop/src/lib/storymap-pdf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* stays free of MapLibre and the DOM and can be unit tested with data-URL
* images.
*/
import jsPDF from "jspdf";
import { jsPDF } from "jspdf";
import { pageMm, resolvePageSize, type Orientation, type PaperSizeId } from "./print-layout";

/** An image to embed: a canvas (app) or a PNG/JPEG data URL (tests), plus its
Expand Down
2 changes: 2 additions & 0 deletions apps/geolibre-desktop/src/main.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import "./lib/symbol-dispose-polyfill";
// Must precede any Map construction (see the module docs).
import "./lib/maplibre-worker";
import React from "react";
import ReactDOM from "react-dom/client";
import "@geoman-io/maplibre-geoman-free/dist/maplibre-geoman.css";
Expand Down
67 changes: 67 additions & 0 deletions apps/geolibre-desktop/vite-plugins/maplibre-default-import-shim.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import type { Plugin } from "vite";

/**
* Rewrite `import X from "maplibre-gl"` to `import * as X from "maplibre-gl"`
* inside the published bundles of packages that still target MapLibre v5.
*
* MapLibre v6 is ESM-only with **no default export**, so a dist file compiled
* against v5's default export is a hard bundling error:
*
* [MISSING_EXPORT] "default" is not exported by ".../maplibre-gl.mjs"
*
* A namespace object is a drop-in for what these bundles actually do with the
* binding (`X.Map`, `new X.Popup()`, …), so the rewrite is safe.
*
* This is a **temporary** shim for third-party packages we do not control. Every
* opengeos-owned package has already been migrated and released, so the list
* below should only ever shrink. See opengeos/GeoLibre#1489 (blocker 1).
*
* The plugin fails the build if a listed package stops matching — that means it
* either shipped a fix (delete the entry) or changed its bundle shape (revisit),
* and silently shimming nothing would hide both.
*/
const SHIMMED_PACKAGES = ["@esri/maplibre-arcgis", "@geoman-io/maplibre-geoman-free"] as const;

// Matches a default import of maplibre-gl in minified or unminified ESM:
// import Zt from"maplibre-gl" import e from "maplibre-gl"
const DEFAULT_IMPORT = /\bimport\s+([A-Za-z_$][\w$]*)\s+from\s*(["'])maplibre-gl\2/g;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Quality/robustness (low-medium confidence): DEFAULT_IMPORT only matches a bare default import (import X from "maplibre-gl"). If either shimmed package's published bundle ever emits a combined default+named import, e.g. import X, { Y } from "maplibre-gl", this regex won't match (no from immediately after the identifier), so the rewrite — and the buildEnd staleness check — would silently miss it, and the build would fail downstream with the raw [MISSING_EXPORT] "default" error instead of the intended clear this.error() message.

tests/maplibre-shim-parity.test.ts only asserts against the two current known bundle shapes, so this would only surface on a future dependency bump that changes the import form. Not a problem today, but worth a short comment noting the regex's coverage is intentionally narrow (or extending it), so it doesn't quietly bit-rot alongside the "list only ever shrinks" invariant this file documents.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This regex only matches a bare default import (import X from "maplibre-gl"). If either shimmed package's bundle ever contains a combined default+named import — import X, { Y } from "maplibre-gl" — this won't rewrite it, and the module still has an unsatisfiable default import going into Rollup.

That in itself would likely fail loudly ([MISSING_EXPORT] "default" is not exported...), so it probably wouldn't ship silently. But the buildEnd staleness guard (lines 56-65) checks only whether the package had at least one rewrite (rewritten.add(pkg)), not whether every matching file in that package was successfully transformed. So if one file in a shimmed package uses the plain form (caught) and another uses the combined form (missed), the guard stays green while the second file still breaks the build — defeating part of the point of the guard, which is to make gaps visible rather than have them surface as an opaque bundler error.

Low confidence this pattern actually appears in either package's current bundle (no way to check node_modules in this environment), but worth a quick check, and maybe worth extending the regex to also match import X, { / import X, * as forms for robustness.


export function maplibreDefaultImportShim(): Plugin {
const rewritten = new Set<string>();

return {
name: "geolibre:maplibre-default-import-shim",
enforce: "pre",
apply: () => true,

transform(code, id) {
const pkg = SHIMMED_PACKAGES.find((name) => id.includes(`/node_modules/${name}/`));
if (!pkg || !code.includes("maplibre-gl")) return null;

DEFAULT_IMPORT.lastIndex = 0;
if (!DEFAULT_IMPORT.test(code)) return null;

rewritten.add(pkg);
DEFAULT_IMPORT.lastIndex = 0;
return {
code: code.replace(
DEFAULT_IMPORT,
(_match, binding: string, quote: string) =>
`import * as ${binding} from ${quote}maplibre-gl${quote}`,
),
map: null,
};
},

buildEnd(error) {
if (error) return;
const stale = SHIMMED_PACKAGES.filter((name) => !rewritten.has(name));
if (stale.length === 0) return;
this.error(
`maplibre default-import shim matched nothing in: ${stale.join(", ")}. ` +
`If the package now ships a v6-compatible build, remove it from ` +
`SHIMMED_PACKAGES in vite-plugins/maplibre-default-import-shim.ts.`,
);
},
};
}
9 changes: 9 additions & 0 deletions apps/geolibre-desktop/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { bundledPlugins } from "./vite-plugins/bundled-plugins";
import { copyCesiumAssets } from "./vite-plugins/copy-cesium-assets";
import { copyRtlText } from "./vite-plugins/copy-rtl-text";
import { copyVectorOps } from "./vite-plugins/copy-vector-ops";
import { maplibreDefaultImportShim } from "./vite-plugins/maplibre-default-import-shim";
import { proxyBinaryRequestGuarded } from "./vite-proxy-guard";

const GEOAGENT_BROWSER_BUNDLE = "maplibre-gl-geoagent/dist/browser-";
Expand Down Expand Up @@ -846,6 +847,7 @@ function pwaPlugin(): Plugin[] {
export default defineConfig({
base: APP_BASE,
plugins: [
maplibreDefaultImportShim(),
...(PGLITE_CDN ? [pgliteCdnLoaderPlugin()] : []),
...(CEREUS_CDN ? [cereusCdnLoaderPlugin()] : []),
duckdbWasmBundlesPlugin(),
Expand Down Expand Up @@ -959,6 +961,13 @@ export default defineConfig({
// asset reference, so serve it as-is. Only reached through the lazy
// dynamic import in local-netcdf.ts when a user opens a local file.
"h5wasm",
// These two still default-import maplibre-gl, which v6 does not provide.
// `maplibreDefaultImportShim` rewrites that, but the dependency optimizer
// runs outside the plugin pipeline and would fail before the shim is ever
// consulted, so serve them unbundled and let the shim transform them.
// Remove alongside their entries in the shim. See opengeos/GeoLibre#1489.
"@esri/maplibre-arcgis",
"@geoman-io/maplibre-geoman-free",
],
},
build: {
Expand Down
Loading
Loading