feat(core,template): <MapPreview> — serverless any-size geo preview via MapLibre + PMTiles (po-6sr.1)#1644
Conversation
…ia MapLibre + PMTiles (po-6sr.1) Phase 1 of the GIS epic (po-6sr §6): kill the "can't preview big GeoJSON" problem with a serverless map preview over a single PMTiles archive. - @portaljs/core: new <MapPreview> (maplibre-gl + pmtiles), exported from the public API. Renders any-size vector PMTiles from R2/any HTTP host via range requests — no tile server. Auto-fits the tileset bounds from the PMTiles header, sensible fill/line/point styling per vector layer, click-to-inspect feature properties, keyless demo-tiles basemap with offline fallback. SSR-safe: maplibre-gl/pmtiles load via dynamic import() in the browser only. - catalog template: 'pmtiles' DataFormat + local MapPreview component wired into the showcase (next/dynamic ssr:false, chunk loads only when a dataset has a PMTiles resource); bundled @reference/world-boundaries demo (Natural Earth 110m countries, public domain, 344 KB, tippecanoe -z5) kept inline per the sample-data fence, with a .gitattributes -text guard for *.pmtiles. - docs: manual tippecanoe→PMTiles ingest path (interim until auto-ingest ships as a later phase) in the template README + <MapPreview> usage in the core README. Verified in a real browser: tiles fetched via 206 range requests, map renders, click popup shows feature properties. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
7 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds PMTiles map preview support in the core package and the catalog example, wires ChangesPMTiles map preview feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
examples/portaljs-catalog/components/MapPreview.tsx (1)
1-389: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffNear-complete duplication of
packages/core'sMapPreview.This component reimplements ~90% of
packages/core/src/ui/MapPreview/MapPreview.tsx(protocol registration, header/metadata reads, layer construction, click-to-inspect, degrade-to-offline logic) rather than consuming the core export. This is presumably driven by the Tailwind-vs-inline-styles guideline split, but it means every future bug fix (e.g., the attribution/PMTiles-instance-sharing issues below) must be applied twice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/portaljs-catalog/components/MapPreview.tsx` around lines 1 - 389, This MapPreview implementation is duplicating most of the logic already present in packages/core’s MapPreview, so update it to reuse the core component or shared helper/hook instead of reimplementing protocol setup, PMTiles loading, layer wiring, and inspection behavior. Keep the portal-specific styling differences isolated, but move the common map lifecycle into the shared MapPreview path so fixes only need to be made once and symbols like MapPreview, injectBaseCss, and the map initialization effect stay aligned.packages/core/src/ui/MapPreview/MapPreview.tsx (1)
113-128: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDuplicate PMTiles instance/requests — share via
protocol.add().A standalone
PMTilesinstance is created to read the header/metadata, but the vector source URLpmtiles://${absoluteUrl}causes the pmtiles protocol to independently resolve its own instance for the same archive, duplicating the initial directory/header fetches. The PMTiles docs recommend registering the instance on the protocol so both paths share one reader.♻️ Share one PMTiles instance
+ const protocol = new pmtiles.Protocol(); const anyGl = maplibregl as unknown as { __portaljsPmtilesProtocol?: boolean; }; if (!anyGl.__portaljsPmtilesProtocol) { - maplibregl.addProtocol("pmtiles", new pmtiles.Protocol().tile); + maplibregl.addProtocol("pmtiles", protocol.tile); anyGl.__portaljsPmtilesProtocol = true; } ... const archive = new pmtiles.PMTiles(absoluteUrl); + protocol.add(archive);Also applies to: 186-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/ui/MapPreview/MapPreview.tsx` around lines 113 - 128, The MapPreview PMTiles setup is creating a separate PMTiles reader for header/metadata while the vector source URL is resolved through the pmtiles protocol, causing duplicate archive requests. In MapPreview.tsx, update the PMTiles initialization around the archive/header loading logic and the source setup so the same `pmtiles.PMTiles` instance is registered through `protocol.add()` and reused by the `pmtiles://${absoluteUrl}` source. Also apply the same shared-instance approach in the later source creation path referenced by the review so both code paths use one reader.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/portaljs-catalog/components/MapPreview.tsx`:
- Around line 94-131: In MapPreview, reuse the same PMTiles instance for both
header/metadata reads and the pmtiles:// vector source by registering the
created archive with protocol.add(archive) before constructing the map, instead
of letting the protocol create a second instance. Also remove the unconditional
attributionControl: false from the map setup so the default basemap attribution
remains visible, since this component always uses the demo basemap and has no
basemap={false} opt-out.
In `@examples/portaljs-catalog/pages/`[owner]/[slug].tsx:
- Around line 27-33: The map preview currently relies on the MapLibre/PMTiles
stack via MapPreview, which conflicts with the approved react-leaflet/leaflet
guideline. Update the implementation around MapPreview and the PMTiles rendering
path to use react-leaflet/leaflet directly for map/GeoJSON rendering, or if
MapLibre + PMTiles is intentionally required, explicitly revise the coding
guidelines to allow this stack. Keep the dynamic client-only loading pattern
intact while swapping the underlying map implementation to the sanctioned
approach.
In `@packages/core/src/ui/MapPreview/MapPreview.tsx`:
- Around line 52-58: The lint failures are coming from missing browser globals
in the ESLint setup, not from the MapPreview logic itself. Update the ESLint
config used by the packages/core scope so DOM symbols referenced in
MapPreview.tsx and related client-only code like document, window, URL, and
HTMLDivElement are recognized (for example by enabling the browser environment
or equivalent DOM globals). Keep the fix in the config rather than changing
injectBaseCss or the other client-side references.
- Around line 140-174: MapPreview is suppressing required basemap attribution by
disabling MapLibre’s attribution control, and the custom attribution prop is
only rendered as plain text. Update MapPreview so the default basemap still
shows MapLibre/OSM attribution (or merge basemap attribution with the caller’s
data-source credit), and ensure the attribution prop in MapPreview is rendered
in a way that preserves any intended link/HTML formatting.
---
Nitpick comments:
In `@examples/portaljs-catalog/components/MapPreview.tsx`:
- Around line 1-389: This MapPreview implementation is duplicating most of the
logic already present in packages/core’s MapPreview, so update it to reuse the
core component or shared helper/hook instead of reimplementing protocol setup,
PMTiles loading, layer wiring, and inspection behavior. Keep the portal-specific
styling differences isolated, but move the common map lifecycle into the shared
MapPreview path so fixes only need to be made once and symbols like MapPreview,
injectBaseCss, and the map initialization effect stay aligned.
In `@packages/core/src/ui/MapPreview/MapPreview.tsx`:
- Around line 113-128: The MapPreview PMTiles setup is creating a separate
PMTiles reader for header/metadata while the vector source URL is resolved
through the pmtiles protocol, causing duplicate archive requests. In
MapPreview.tsx, update the PMTiles initialization around the archive/header
loading logic and the source setup so the same `pmtiles.PMTiles` instance is
registered through `protocol.add()` and reused by the `pmtiles://${absoluteUrl}`
source. Also apply the same shared-instance approach in the later source
creation path referenced by the review so both code paths use one reader.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f3293f2c-d577-4b90-9e4f-81ff1d592719
⛔ Files ignored due to path filters (2)
examples/portaljs-catalog/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
examples/portaljs-catalog/.gitattributesexamples/portaljs-catalog/README.mdexamples/portaljs-catalog/components/MapPreview.tsxexamples/portaljs-catalog/datasets.jsonexamples/portaljs-catalog/lib/providers/types.tsexamples/portaljs-catalog/package.jsonexamples/portaljs-catalog/pages/[owner]/[slug].tsxexamples/portaljs-catalog/public/data/world-boundaries.pmtilespackages/core/README.mdpackages/core/package.jsonpackages/core/src/ui/MapPreview/MapPreview.tsxpackages/core/src/ui/MapPreview/index.tspackages/core/src/ui/index.ts
| const archive = new pmtiles.PMTiles(absoluteUrl) | ||
| let header: Awaited<ReturnType<typeof archive.getHeader>> | ||
| let vectorLayers: { id: string }[] | ||
| try { | ||
| header = await archive.getHeader() | ||
| const metadata = (await archive.getMetadata()) as { | ||
| vector_layers?: { id: string }[] | ||
| } | ||
| vectorLayers = metadata?.vector_layers ?? [] | ||
| } catch { | ||
| if (!cancelled) { | ||
| setError(`Could not read the PMTiles archive at ${url}.`) | ||
| setLoading(false) | ||
| } | ||
| return | ||
| } | ||
| if (cancelled || !containerRef.current) return | ||
|
|
||
| const offlineStyle: StyleSpecification = { | ||
| version: 8, | ||
| sources: {}, | ||
| layers: [ | ||
| // Matches the cream-panel canvas so a basemap-less map still looks placed. | ||
| { id: 'background', type: 'background', paint: { 'background-color': '#ece7db' } }, | ||
| ], | ||
| } | ||
|
|
||
| try { | ||
| map = new maplibregl.Map({ | ||
| container: containerRef.current, | ||
| style: BASEMAP_STYLE, | ||
| center: [ | ||
| (header.minLon + header.maxLon) / 2, | ||
| (header.minLat + header.maxLat) / 2, | ||
| ], | ||
| zoom: 1, | ||
| attributionControl: false, | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Same duplicate-PMTiles-instance and dropped-attribution issues as the core component.
- Lines 94-131: a standalone
PMTilesinstance reads header/metadata, while thepmtiles://${absoluteUrl}vector source (Line 161) causes the protocol to create a second instance for the same archive — share viaprotocol.add(archive)as flagged inpackages/core/src/ui/MapPreview/MapPreview.tsx(Lines 113-128). - Line 130:
attributionControl: falsedrops the default basemap's required OSM attribution; this component has nobasemap={false}escape hatch either, so it always shows the demo basemap without attribution.
Also applies to: 160-162
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/portaljs-catalog/components/MapPreview.tsx` around lines 94 - 131,
In MapPreview, reuse the same PMTiles instance for both header/metadata reads
and the pmtiles:// vector source by registering the created archive with
protocol.add(archive) before constructing the map, instead of letting the
protocol create a second instance. Also remove the unconditional
attributionControl: false from the map setup so the default basemap attribution
remains visible, since this component always uses the demo basemap and has no
basemap={false} opt-out.
| // MapLibre GL touches `window` at module scope, so the map preview is likewise | ||
| // client-only. The chunk (maplibre + pmtiles) loads only when a dataset | ||
| // actually has a PMTiles resource. | ||
| const MapPreview = dynamic(() => import('../../components/MapPreview'), { | ||
| ssr: false, | ||
| }) | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Map stack deviates from coding guidelines (MapLibre/PMTiles vs. react-leaflet/leaflet).
The comment confirms MapLibre GL is used for map rendering, and the MapPreview component (loaded here and rendered for pmtiles resources) is built on maplibre-gl + pmtiles. The coding guidelines mandate react-leaflet/leaflet directly for GeoJSON/map rendering. This PR introduces a different map stack entirely, which the guidelines don't sanction.
If MapLibre + PMTiles is intentionally chosen for vector-tile/PMTiles support (which react-leaflet doesn't natively provide), the guideline should be updated to reflect the new approved stack; otherwise the map rendering should be reworked onto react-leaflet/leaflet per the existing policy.
As per coding guidelines: "Add react-leaflet and leaflet directly for GeoJSON/map rendering; do not bundle or import map components from @portaljs/components."
Also applies to: 448-452
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/portaljs-catalog/pages/`[owner]/[slug].tsx around lines 27 - 33, The
map preview currently relies on the MapLibre/PMTiles stack via MapPreview, which
conflicts with the approved react-leaflet/leaflet guideline. Update the
implementation around MapPreview and the PMTiles rendering path to use
react-leaflet/leaflet directly for map/GeoJSON rendering, or if MapLibre +
PMTiles is intentionally required, explicitly revise the coding guidelines to
allow this stack. Keep the dynamic client-only loading pattern intact while
swapping the underlying map implementation to the sanctioned approach.
Source: Coding guidelines
| function injectBaseCss() { | ||
| const id = "portaljs-map-preview-css"; | ||
| if (document.getElementById(id)) return; | ||
| const style = document.createElement("style"); | ||
| style.id = id; | ||
| style.textContent = BASE_CSS; | ||
| document.head.appendChild(style); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
ESLint no-undef errors for DOM globals will likely fail lint in CI.
Static analysis flags document, HTMLDivElement, URL, and window as undefined. The code is correct (these only run client-side inside useEffect), but the eslint config for this file/package appears to be missing browser globals (e.g., env: browser or DOM lib globals). This is a config gap, not a logic bug, but will surface as real lint failures.
🔧 Likely fix (adjust eslint config, not this file)
// .eslintrc.js (or equivalent) for packages/core
module.exports = {
env: { browser: true, es2021: true },
// ...
};Also applies to: 77-77, 111-111
🧰 Tools
🪛 ESLint
[error] 54-54: 'document' is not defined.
(no-undef)
[error] 55-55: 'document' is not defined.
(no-undef)
[error] 58-58: 'document' is not defined.
(no-undef)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/ui/MapPreview/MapPreview.tsx` around lines 52 - 58, The
lint failures are coming from missing browser globals in the ESLint setup, not
from the MapPreview logic itself. Update the ESLint config used by the
packages/core scope so DOM symbols referenced in MapPreview.tsx and related
client-only code like document, window, URL, and HTMLDivElement are recognized
(for example by enabling the browser environment or equivalent DOM globals).
Keep the fix in the config rather than changing injectBaseCss or the other
client-side references.
Source: Linters/SAST tools
| map = new maplibregl.Map({ | ||
| container: containerRef.current, | ||
| style: basemap === false ? offlineStyle : basemap, | ||
| center: center ?? [ | ||
| (header.minLon + header.maxLon) / 2, | ||
| (header.minLat + header.maxLat) / 2, | ||
| ], | ||
| zoom: zoom ?? 1, | ||
| attributionControl: false, | ||
| }); | ||
| } catch (e) { | ||
| // Most commonly WebGL being unavailable (old hardware, disabled in | ||
| // the browser) — say so instead of a generic failure. | ||
| if (!cancelled) setError(e instanceof Error ? e.message : "Could not initialize the map."); | ||
| return; | ||
| } | ||
| mapRef.current = map; | ||
|
|
||
| // A missing/unreachable basemap must never take the data layer down | ||
| // with it: swap to the offline background and carry on. Match only | ||
| // network-shaped failures before the style is up — internal maplibre | ||
| // errors (e.g. "Style is not done loading") must NOT trigger it. | ||
| let degraded = false; | ||
| map.on("error", (e: { error?: Error & { status?: number } }) => { | ||
| const message = e.error?.message ?? ""; | ||
| if ( | ||
| !degraded && | ||
| basemap !== false && | ||
| !map?.isStyleLoaded() && | ||
| /fetch|network|ajax/i.test(message) | ||
| ) { | ||
| degraded = true; | ||
| map?.setStyle(offlineStyle); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Default demo basemap attribution is silently dropped.
attributionControl: false disables MapLibre's built-in attribution UI entirely. The default basemap (demotiles.maplibre.org) is backed by OpenStreetMap-derived data whose licensing expects visible attribution, and the attribution prop is documented purely as the caller's data-source credit, not basemap credit — so when a caller uses the default basemap and sets attribution="Natural Earth", no OSM/MapLibre attribution is shown anywhere. Also note the attribution prop is rendered as a plain text node (Line 453), so even an HTML attribution string (as pmtiles/MapLibre examples recommend) won't render as a link.
🛠️ Keep attribution control (or merge basemap + data attribution)
- map = new maplibregl.Map({
- container: containerRef.current,
- style: basemap === false ? offlineStyle : basemap,
- center: center ?? [
- (header.minLon + header.maxLon) / 2,
- (header.minLat + header.maxLat) / 2,
- ],
- zoom: zoom ?? 1,
- attributionControl: false,
- });
+ map = new maplibregl.Map({
+ container: containerRef.current,
+ style: basemap === false ? offlineStyle : basemap,
+ center: center ?? [
+ (header.minLon + header.maxLon) / 2,
+ (header.minLat + header.maxLat) / 2,
+ ],
+ zoom: zoom ?? 1,
+ attributionControl: basemap !== false,
+ });Also applies to: 438-455
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/ui/MapPreview/MapPreview.tsx` around lines 140 - 174,
MapPreview is suppressing required basemap attribution by disabling MapLibre’s
attribution control, and the custom attribution prop is only rendered as plain
text. Update MapPreview so the default basemap still shows MapLibre/OSM
attribution (or merge basemap attribution with the caller’s data-source credit),
and ensure the attribution prop in MapPreview is rendered in a way that
preserves any intended link/HTML formatting.
… (po-6sr.1) Migrate the two bundled binary samples (region-metrics.parquet, world-boundaries.pmtiles) off raw git commit onto R2 via Giftless, referenced by data.portaljs.com URLs, so the showcase demonstrates the real serverless range-read path and the repo carries no binary blobs. CSVs stay inline for zero-cred offline dev. Updates .gitattributes, datasets.json, README. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Phase 1 of the GIS epic po-6sr (§6): a serverless map preview that kills the "can't preview big GeoJSON" problem. Bead: po-6sr.1. Do not merge — mayor lands.
What
@portaljs/core: new<MapPreview>(maplibre-gl + pmtiles, exported from the public API). Renders any-size vector PMTiles from R2 / any static HTTP host via HTTP range requests — only the tiles in view are fetched, no tile server.center/zoom).basemap={false}for fully offline); basemap failure never takes the data layer down.maplibre-gl/pmtilesload via dynamicimport()insideuseEffect, so builds (including CI "Build site") never touchwindow.pmtilesadded toDataFormat; localcomponents/MapPreview.tsxwired into the showcase vianext/dynamicssr:false(the maplibre chunk loads only when a dataset actually has a PMTiles resource).@reference/world-boundaries— Natural Earth 110m countries (public domain), tiled withtippecanoe -z5to a 344 KB archive, committed inline underpublic/data/per the sample-data fence, with a.gitattributes*.pmtiles -textguard.tippecanoe → PMTilesingest path in the template README ("Map tier" section) +<MapPreview>usage in the core README. Auto-ingest is a later phase (separate bead), per the epic.Verification
Drove the built template in a real (headed) browser:
206 Partial Contentrange requests (header, then per-tile).Local CI parity: all three packages build,
@portaljs/ckanlint clean, templatenext buildgreen (demo page prerendered).Out of scope (later phases)
Auto-ingest pipeline (container compute), GeoParquet + DuckDB spatial (
<GeoQuery>), COG/raster, Koop/OGC adapters.🤖 Generated with Claude Code
Summary by CodeRabbit
pmtilesformat and a world boundaries PMTiles sample dataset.