-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add basemap-browser example application #9892
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.
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
Conversation
- Comprehensive test application for deck.gl with multiple basemap providers - Supports Google Maps, Mapbox, MapLibre, and MapLibre Globe - Both Pure JS and React implementations for complete coverage - Interleaved mode toggle for testing shared vs separate GL contexts - Real-time metrics panel showing DPR, canvas dimensions, and drawing buffer - Complete isolation between Pure JS and React implementations - Fixed globe projection in interleaved mode (set projection before overlay) - Labels render above deck.gl layers in interleaved mode using beforeId/slot - TypeScript throughout with full type safety - Covers all 16 test scenarios from resize/DPR bug fix
- Pure JS code now mounts directly to DOM with no React wrapper - Control panel is separate React root that calls callback function - Map area is either Pure JS mount OR separate React root - Complete isolation between Pure JS and React implementations - Removed app.tsx, map-container.tsx, pure-js-container.tsx - Created control-panel.tsx standalone component - Created examples-react/ directory with separate components: - google-maps-component.tsx - mapbox-component.tsx - maplibre-component.tsx - Updated index.html with side-by-side layout (#controls and #map divs) - index.tsx orchestrates two separate React roots - No more React wrapping Pure JS code - true isolation achieved
- Defer cleanup and remounting to next tick using setTimeout - Prevents synchronous unmount during React render phase - Fixes 'Attempted to synchronously unmount a root' warning - Fixes 'Failed to execute removeChild' error - Extracted mountExample function for cleaner separation
felixpalmer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Given that we have a screenshot widget now, would it be possible to somehow run through all the variants and display a set of thumbnails as a sort of smoke test? Alternatively, could we somehow include these in our automated tests?
I think this is sort of possible! We could add the line of code from the screenshot widget that captures the canvas, and collect screenshots. Similar to a render test it'd go one by one through the examples to collect the results. This would really only be applicable to interleaved renders since those can capture both basemap and deck. Additionally, we could add a maplibre-based render test for the globe and Mercator variants (the others would require API keys to run the tests, which doesn't sound great) |
|
I gave the thumbnails idea a try and ran into issues where the images were empty unless I was actively interacting with the maps. I think something is clearing the buffers, but I'm not sure what since I've tried all of the usual suspects. I'm going to merge this in for now - happy to share what I tried if you're curious and want to give it a shot |
examples/basemap-browser/src/examples-react/mapbox-component.tsx
Outdated
Show resolved
Hide resolved
examples/basemap-browser/src/examples-react/maplibre-component.tsx
Outdated
Show resolved
Hide resolved
|
@copilot I left a series of comments on this PR I'd like you to address |
|
@chrisgervang I've opened a new pull request, #9934, to work on those changes. Once the pull request is ready, I'll request review from you. |
Fixed logic that incorrectly applied MapLibre-specific `beforeId` prop to Google Maps layers. Google Maps doesn't support layer positioning props like `beforeId` or `slot` in interleaved mode. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changed cleanup function to use overlay.finalize() instead of overlay.setMap(null) for proper cleanup of the GoogleMapsOverlay. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added cancellation mechanisms to prevent async callbacks from running
after cleanup:
- maplibre.ts: Added cancelled flag to prevent map.on('load') callback
from executing after cleanup, matching the pattern used in google-maps.ts
- maplibre-component.tsx: Added isMountedRef to prevent state updates
on unmounted components in the onLoad callback
This prevents potential errors when components unmount before maps finish
loading.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
| // Mount new example | ||
| mountExample(example, interleaved); | ||
| }, 0); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing cancellation flag for rapid example switching
Medium Severity
The loadExample function uses setTimeout to defer cleanup but lacks a cancellation mechanism. When called rapidly (e.g., user quickly switching between examples), each call schedules a separate timeout, causing all intermediate examples to be mounted and immediately cleaned up. This results in unnecessary work and potential visual flickering. The pure JS mount functions already implement cancellation patterns with cancelled flags, but the orchestrating loadExample function does not.
| return () => { | ||
| cancelled = true; | ||
| map.remove(); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overlay created before cancellation check may leak resources
Low Severity
The deckOverlay is created at line 27 before the cancellation check, but only added to the map inside the load event handler after checking cancelled. If cleanup runs before the map finishes loading, the overlay is never added to the map and never cleaned up since the cleanup function only calls map.remove(). This is inconsistent with google-maps.ts which creates the overlay only after verifying the operation wasn't cancelled, preventing orphaned resources.
Summary
This PR adds a comprehensive basemap-browser example application for testing deck.gl with multiple basemap providers across different frameworks and rendering modes.
Features
Use Case
This application makes it easy to test the resize/DPR bug fix across all 16+ test scenarios:
Architecture Highlights
.mount()functions, no React in call stackexamples-react/directoryTechnical Details
beforeId: 'watername_ocean'for MapLibre andslot: 'middle'for MapboxNote
Introduces a self-contained example app to quickly exercise deck.gl against multiple basemaps, frameworks, and rendering modes.
examples/basemap-browserapp (Vite) with React control panel and separate map rootconstants,layers(airport + arcs) with interleaved positioning (slotfor Mapbox,beforeIdfor MapLibre)Written by Cursor Bugbot for commit c45a863. This will update automatically on new commits. Configure here.