Clean up basemap-browser example per review feedback #9934

Copilot · 2026-01-12T08:51:20Z

Background

Address code review feedback on the basemap-browser example application.

Change List

Naming & Consistency

Rename overlay components: MapboxOverlayWrapper → MapboxDeckOverlay, MapLibreOverlay → MapLibreDeckOverlay
Use caret range ^9.2.0 for deck.gl submodule packages (@deck.gl/core, @deck.gl/google-maps, @deck.gl/layers, @deck.gl/mapbox)
Remove redundant deck.gl package dependency (code only uses individual submodules)

Cleanup & Optimization

Remove redundant deckOverlay.finalize() in mapbox/maplibre Pure JS implementations (map.remove() handles cleanup via onRemove)
Remove duplicate initial load useEffect in control-panel.tsx
Add cancellation flag to google-maps.ts async API loading to prevent post-unmount state updates
Remove unnecessary wrapper comments from React components

Configuration & Documentation

Extend root tsconfig.json instead of duplicating compiler options
Update README architecture section to accurately describe Pure JS and React integration patterns
Format test matrix as table (16 configurations: 4 providers × 2 frameworks × 2 interleaved modes)
Remove redundant "Status" column from test matrix

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Note

Streamlines the basemap-browser example and tightens docs/config.

Dependencies: Pin @deck.gl/* to ^9.2.0, remove deck.gl, add vite dev dep in package.json
React components: Rename overlays to MapboxDeckOverlay and MapLibreDeckOverlay
Teardown/async: Remove redundant deckOverlay.finalize() in Pure JS mapbox.ts/maplibre.ts; add cancellation guard to examples-pure-js/google-maps.ts
Control panel: Drop duplicate initial-load useEffect
Config: tsconfig.json now extends root and trims options
Docs: Update README.md architecture and format test matrix as a table

^{Written by Cursor Bugbot for commit aa849e3. This will update automatically on new commits. Configure here.}

Co-authored-by: chrisgervang <[email protected]>

chrisgervang · 2026-01-12T09:02:39Z

examples/basemap-browser/README.md

- ✅ Pure JS + Interleaved: false
- ✅ React + Interleaved: true
- ✅ React + Interleaved: false
+| Provider | Framework | Interleaved | Status |


Remove status column

chrisgervang · 2026-01-12T09:02:55Z

examples/basemap-browser/README.md

 ## Architecture

-The basemap-browser uses TypeScript and React function components with a modular architecture:
+The basemap-browser uses a two-React-roots architecture with TypeScript throughout:


This sentence doesn't make sense

chrisgervang · 2026-01-12T09:03:35Z

examples/basemap-browser/package.json

    "@deck.gl/layers": "*",
    "@deck.gl/mapbox": "*",
-    "@vis.gl/react-google-maps": "^1.7.1",
+    "@vis.gl/react-google-maps": "1.7.1",


I didn't mean you needed to pin react-google-maps. I meant you needed to pin deck.gl packages

chrisgervang · 2026-01-12T09:04:02Z

examples/basemap-browser/src/examples-react/mapbox-component.tsx

 // eslint-disable-next-line no-process-env
 const MAPBOX_TOKEN = process.env.MapboxAccessToken;

 // Mapbox Overlay wrapper


Remove comment

chrisgervang · 2026-01-12T09:04:14Z

examples/basemap-browser/src/examples-react/maplibre-component.tsx


 import 'maplibre-gl/dist/maplibre-gl.css';

 // MapLibre Overlay wrapper


Remove comment

chrisgervang · 2026-01-12T09:06:21Z

@copilot please take a look at my comments

Co-authored-by: chrisgervang <[email protected]>

Copilot · 2026-01-12T09:08:17Z

@copilot please take a look at my comments

All comments addressed in commit d6b69d2:

✅ Removed "Status" column from test matrix table
✅ Simplified architecture description (removed "with TypeScript throughout")
✅ Pinned deck.gl packages to 9.0.33, reverted react-google-maps to ^1.7.1
✅ Removed "Mapbox Overlay wrapper" comment
✅ Removed "MapLibre Overlay wrapper" comment

chrisgervang · 2026-01-12T09:19:36Z

examples/basemap-browser/README.md

 ## Architecture

-The basemap-browser uses TypeScript and React function components with a modular architecture:
+The basemap-browser uses a two-React-roots architecture:


This still isn't true or a correct summary of the architecture.

Fixed in commit 2990945. Changed to "This example demonstrates both Pure JS and React integration patterns with deck.gl" which accurately describes the application structure.

chrisgervang · 2026-01-12T09:19:58Z

examples/basemap-browser/package.json

-    "@deck.gl/google-maps": "*",
-    "@deck.gl/layers": "*",
-    "@deck.gl/mapbox": "*",
+    "@deck.gl/core": "9.0.33",


These deck modules need to be the latest 9.2

Updated to 9.2.0-beta.4 in commit 2990945.

chrisgervang · 2026-01-12T09:20:53Z

@copilot I left a review

Co-authored-by: chrisgervang <[email protected]>

chrisgervang · 2026-01-12T16:24:08Z

examples/basemap-browser/package.json

-    "@deck.gl/google-maps": "*",
-    "@deck.gl/layers": "*",
-    "@deck.gl/mapbox": "*",
+    "@deck.gl/core": "9.2.0-beta.4",


It's ^9.2.0

Updated to ^9.2.0 in commit aa849e3.

chrisgervang · 2026-01-12T16:25:10Z

examples/basemap-browser/package.json

+    "@deck.gl/mapbox": "9.2.0-beta.4",
    "@vis.gl/react-google-maps": "^1.7.1",
-    "deck.gl": "*",
+    "deck.gl": "9.2.0-beta.4",


Why do we need this when we're already adding submodules?

Removed in commit aa849e3. The code only imports from @deck.gl/* submodules, so the main deck.gl package was redundant.

chrisgervang · 2026-01-12T16:25:54Z

@copilot I've left comments

…pendency Co-authored-by: chrisgervang <[email protected]>

* Add basemap-browser example application - 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 * Refactor basemap-browser to use multiple React roots architecture - 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 * Update index.html * Fix race condition when switching between React and Pure JS examples - 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 * Update README.md * Update README.md * Clean up basemap-browser example per review feedback (#9934) * fix(basemap-browser): correct interleaved props for Google Maps layers 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 <[email protected]> * fix(basemap-browser): use finalize() for Google Maps overlay cleanup 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 <[email protected]> * fix(basemap-browser): add cancellation flags to MapLibre examples 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 <[email protected]> --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Claude Sonnet 4.5 <[email protected]>

Initial plan

cadc25a

Copilot AI assigned Copilot and chrisgervang Jan 12, 2026

Copilot AI mentioned this pull request Jan 12, 2026

Add basemap-browser example application #9892

Merged

Copilot started work on behalf of chrisgervang January 12, 2026 08:51 View session

Address PR review comments

3c7f6f3

Co-authored-by: chrisgervang <[email protected]>

Copilot AI changed the title ~~[WIP] Add basemap-browser example application~~ Clean up basemap-browser example per review feedback Jan 12, 2026

Copilot AI requested a review from chrisgervang January 12, 2026 08:59

Copilot finished work on behalf of chrisgervang January 12, 2026 08:59

chrisgervang reviewed Jan 12, 2026

View reviewed changes

Copilot started work on behalf of chrisgervang January 12, 2026 09:06 View session

Address additional review feedback

d6b69d2

Co-authored-by: chrisgervang <[email protected]>

Copilot AI requested a review from chrisgervang January 12, 2026 09:09

Copilot finished work on behalf of chrisgervang January 12, 2026 09:09

chrisgervang requested changes Jan 12, 2026

View reviewed changes

Copilot started work on behalf of chrisgervang January 12, 2026 09:20 View session

Fix architecture description and update deck.gl to 9.2.0-beta.4

2990945

Co-authored-by: chrisgervang <[email protected]>

Copilot AI requested a review from chrisgervang January 12, 2026 09:23

Copilot finished work on behalf of chrisgervang January 12, 2026 09:23

Copilot started work on behalf of chrisgervang January 12, 2026 09:23 View session

Copilot finished work on behalf of chrisgervang January 12, 2026 09:24

chrisgervang requested changes Jan 12, 2026

View reviewed changes

Copilot started work on behalf of chrisgervang January 12, 2026 16:25 View session

Use caret ranges for deck.gl packages and remove redundant deck.gl de…

aa849e3

…pendency Co-authored-by: chrisgervang <[email protected]>

Copilot AI requested a review from chrisgervang January 12, 2026 16:28

Copilot finished work on behalf of chrisgervang January 12, 2026 16:28

Copilot started work on behalf of chrisgervang January 12, 2026 16:28 View session

chrisgervang approved these changes Jan 12, 2026

View reviewed changes

chrisgervang marked this pull request as ready for review January 12, 2026 16:29

chrisgervang merged commit be29e17 into basemap-browser Jan 12, 2026
4 of 5 checks passed

chrisgervang deleted the copilot/sub-pr-9892 branch January 12, 2026 16:29

Copilot AI restored the copilot/sub-pr-9892 branch January 12, 2026 16:30

Copilot AI requested a review from chrisgervang January 12, 2026 16:30

Copilot stopped work on behalf of chrisgervang due to an error January 12, 2026 16:30
The session was cancelled by the user.


		import 'maplibre-gl/dist/maplibre-gl.css';

		// MapLibre Overlay wrapper

Clean up basemap-browser example per review feedback #9934

Clean up basemap-browser example per review feedback #9934

Uh oh!

Conversation

Copilot AI commented Jan 12, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Change List

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisgervang commented Jan 12, 2026

Uh oh!

Copilot AI commented Jan 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

chrisgervang commented Jan 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

chrisgervang commented Jan 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Jan 12, 2026 •

edited by cursor bot

Loading