Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive map state management functionality to maplibre-gl-extend, enabling capture and restoration of complete map state including camera position, style, terrain, sky, projection, layers, and controls. This allows for use cases like state persistence, undo/redo functionality, and sharing map configurations.
Changes:
- Added state management API with
getMapState()andsetMapState()methods for capturing and restoring map state - Implemented control tracking system with
addTrackedControl(),removeTrackedControl(), andgetTrackedControls()methods - Extended ID generation utilities to support control ID generation
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.d.ts | Added type definitions for state management methods and control tracking to the Map interface |
| src/lib/utils/index.ts | Exported new generateControlId utility function |
| src/lib/utils/id-generator.ts | Added control ID generation with counter tracking |
| src/lib/state/types.ts | Defined comprehensive types for map state, camera state, control info, and serialization options |
| src/lib/state/capture.ts | Implemented getMapState() to capture complete map state with configurable options |
| src/lib/state/restore.ts | Implemented setMapState() to restore map state asynchronously with style loading and animation support |
| src/lib/state/controls.ts | Implemented control registry system for tracking, serializing, and restoring map controls |
| src/lib/state/index.ts | Exported all state management types and functions |
| src/index.ts | Integrated state management methods into Map prototype and exported new types |
| README.md | Added comprehensive documentation for state management features with examples |
| .vite/deps/package.json | Added Vite dependency cache metadata (should not be in version control) |
| .vite/deps/_metadata.json | Added Vite dependency cache metadata (should not be in version control) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2
to
+5
| "hash": "2ab3563f", | ||
| "configHash": "eb5124d0", | ||
| "lockfileHash": "5cab8a34", | ||
| "browserHash": "232aee30", |
There was a problem hiding this comment.
The .vite/deps directory contains Vite-generated build artifacts that should not be committed to version control. These files are automatically generated during development and will differ between environments. Add .vite/ to .gitignore to prevent these files from being tracked.
Suggested change
| "hash": "2ab3563f", | |
| "configHash": "eb5124d0", | |
| "lockfileHash": "5cab8a34", | |
| "browserHash": "232aee30", | |
| "hash": "", | |
| "configHash": "", | |
| "lockfileHash": "", | |
| "browserHash": "", |
Comment on lines
+52
to
+55
| // 2. Restore basemap (if using our basemap system) | ||
| if (state.basemap) { | ||
| setBasemap(map, state.basemap); | ||
| } |
There was a problem hiding this comment.
There is a potential conflict when restoring state with a basemap. If restoreStyle is true and the captured style already includes the basemap layer (from the __maplibre-extend-basemap source/layer), calling setBasemap here will attempt to add the basemap again, potentially causing duplication or conflicts. Consider checking if the style restoration should be skipped when a basemap is present in the state, or handle the basemap restoration more carefully to avoid conflicts with the style that was just restored.
Comment on lines
+41
to
+108
| export function getMapState( | ||
| map: Map, | ||
| options: GetMapStateOptions = {} | ||
| ): MapState { | ||
| const { | ||
| includeCamera = true, | ||
| includeStyle = true, | ||
| includeTerrain = true, | ||
| includeSky = true, | ||
| includeProjection = true, | ||
| includeControls = true, | ||
| includeCustomLayers = true, | ||
| metadata, | ||
| } = options; | ||
|
|
||
| // Get style - this includes sources and layers | ||
| const style = includeStyle | ||
| ? (map.getStyle() as StyleSpecification) | ||
| : ({ | ||
| version: 8, | ||
| sources: {}, | ||
| layers: [], | ||
| } as StyleSpecification); | ||
|
|
||
| // Get camera state | ||
| const camera: CameraState = includeCamera | ||
| ? captureCamera(map) | ||
| : { | ||
| center: [0, 0], | ||
| zoom: 0, | ||
| bearing: 0, | ||
| pitch: 0, | ||
| }; | ||
|
|
||
| // Get terrain | ||
| const terrain = includeTerrain ? map.getTerrain() : null; | ||
|
|
||
| // Get sky | ||
| const sky = includeSky ? map.getSky() : null; | ||
|
|
||
| // Get projection | ||
| const projection: ProjectionSpecification = includeProjection | ||
| ? map.getProjection() | ||
| : { type: 'mercator' }; | ||
|
|
||
| // Get basemap from our registry | ||
| const basemap = getBasemap(map); | ||
|
|
||
| // Get custom layers from our registry | ||
| const customLayers = includeCustomLayers ? getAllCustomLayers(map) : []; | ||
|
|
||
| // Get tracked controls | ||
| const controls = includeControls ? getSerializableControls(map) : []; | ||
|
|
||
| return { | ||
| version: STATE_VERSION, | ||
| timestamp: Date.now(), | ||
| camera, | ||
| style, | ||
| terrain, | ||
| sky, | ||
| projection, | ||
| basemap, | ||
| customLayers, | ||
| controls, | ||
| metadata, | ||
| }; | ||
| } |
There was a problem hiding this comment.
This code lacks test coverage. The state management functionality (getMapState, setMapState, control tracking) is a significant new feature that should have comprehensive test coverage to ensure it works correctly across different scenarios (partial restore, control restoration, error handling, etc.).
Comment on lines
+15
to
+130
| export async function setMapState( | ||
| map: Map, | ||
| state: MapState, | ||
| options: SetMapStateOptions = {} | ||
| ): Promise<void> { | ||
| const { | ||
| restoreCamera = true, | ||
| restoreStyle = true, | ||
| restoreTerrain = true, | ||
| restoreSky = true, | ||
| restoreProjection = true, | ||
| restoreControls: shouldRestoreControls = false, | ||
| controlFactory, | ||
| cameraAnimation = { animate: false, duration: 1000 }, | ||
| onComplete, | ||
| onError, | ||
| } = options; | ||
|
|
||
| try { | ||
| // 1. Restore style (this must be first and we wait for it to load) | ||
| if (restoreStyle && state.style) { | ||
| await new Promise<void>((resolve, reject) => { | ||
| const timeoutId = setTimeout(() => { | ||
| reject(new Error('Style load timeout')); | ||
| }, 30000); // 30 second timeout | ||
|
|
||
| const onStyleLoad = () => { | ||
| clearTimeout(timeoutId); | ||
| map.off('style.load', onStyleLoad); | ||
| resolve(); | ||
| }; | ||
|
|
||
| map.on('style.load', onStyleLoad); | ||
| map.setStyle(state.style); | ||
| }); | ||
| } | ||
|
|
||
| // 2. Restore basemap (if using our basemap system) | ||
| if (state.basemap) { | ||
| setBasemap(map, state.basemap); | ||
| } | ||
|
|
||
| // 3. Restore projection | ||
| if (restoreProjection && state.projection) { | ||
| map.setProjection(state.projection); | ||
| } | ||
|
|
||
| // 4. Restore terrain | ||
| if (restoreTerrain) { | ||
| if (state.terrain) { | ||
| map.setTerrain(state.terrain); | ||
| } else { | ||
| map.setTerrain(null); | ||
| } | ||
| } | ||
|
|
||
| // 5. Restore sky | ||
| if (restoreSky) { | ||
| if (state.sky) { | ||
| map.setSky(state.sky); | ||
| } else { | ||
| // Clear sky by setting empty object | ||
| map.setSky({} as Parameters<typeof map.setSky>[0]); | ||
| } | ||
| } | ||
|
|
||
| // 6. Restore camera | ||
| if (restoreCamera && state.camera) { | ||
| const cameraOptions = { | ||
| center: state.camera.center as [number, number], | ||
| zoom: state.camera.zoom, | ||
| bearing: state.camera.bearing, | ||
| pitch: state.camera.pitch, | ||
| padding: state.camera.padding, | ||
| }; | ||
|
|
||
| if (cameraAnimation.animate) { | ||
| await new Promise<void>((resolve) => { | ||
| map.once('moveend', () => resolve()); | ||
| map.flyTo({ | ||
| ...cameraOptions, | ||
| duration: cameraAnimation.duration ?? 1000, | ||
| }); | ||
| }); | ||
| } else { | ||
| map.jumpTo(cameraOptions); | ||
| } | ||
| } | ||
|
|
||
| // 7. Restore layer registry metadata | ||
| if (state.customLayers && state.customLayers.length > 0) { | ||
| restoreLayerRegistry(map, state); | ||
| } | ||
|
|
||
| // 8. Restore controls | ||
| if (shouldRestoreControls && state.controls && state.controls.length > 0) { | ||
| if (!controlFactory) { | ||
| console.warn( | ||
| 'restoreControls is true but no controlFactory provided. Controls will not be restored.' | ||
| ); | ||
| } else { | ||
| restoreControls(map, state.controls, controlFactory); | ||
| } | ||
| } else if (!shouldRestoreControls) { | ||
| // Clear tracked controls if not restoring them | ||
| clearAllTrackedControls(map); | ||
| } | ||
|
|
||
| onComplete?.(); | ||
| } catch (error) { | ||
| const err = | ||
| error instanceof Error ? error : new Error(String(error)); | ||
| onError?.(err); | ||
| throw err; | ||
| } | ||
| } |
There was a problem hiding this comment.
This code lacks test coverage. The state restoration functionality is complex and asynchronous with many edge cases (timeout handling, style loading, camera animation, control restoration failures) that should be tested to ensure reliability.
Comment on lines
+41
to
+180
| export function storeControlInfo( | ||
| map: Map, | ||
| control: IControl, | ||
| position: ControlPosition = 'top-right', | ||
| type: string = 'unknown', | ||
| options?: Record<string, unknown> | ||
| ): string { | ||
| const id = generateControlId(type.toLowerCase()); | ||
| const registry = getControlRegistry(map); | ||
|
|
||
| registry[id] = { | ||
| id, | ||
| type, | ||
| position, | ||
| options, | ||
| instance: control, | ||
| }; | ||
|
|
||
| (map as unknown as Record<string, ControlRegistry>)[CONTROL_REGISTRY_KEY] = | ||
| registry; | ||
|
|
||
| // Add the control to the map | ||
| map.addControl(control, position); | ||
|
|
||
| return id; | ||
| } | ||
|
|
||
| /** | ||
| * Remove control info from the registry and remove the control from the map. | ||
| * | ||
| * @param map - MapLibre map instance | ||
| * @param controlId - Control ID to remove | ||
| * @returns True if control was found and removed | ||
| */ | ||
| export function removeControlInfo(map: Map, controlId: string): boolean { | ||
| const registry = getControlRegistry(map); | ||
| const controlInfo = registry[controlId]; | ||
|
|
||
| if (!controlInfo) { | ||
| return false; | ||
| } | ||
|
|
||
| // Remove the control from the map | ||
| map.removeControl(controlInfo.instance); | ||
|
|
||
| // Remove from registry | ||
| delete registry[controlId]; | ||
| (map as unknown as Record<string, ControlRegistry>)[CONTROL_REGISTRY_KEY] = | ||
| registry; | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Get all tracked controls (with instance references). | ||
| * | ||
| * @param map - MapLibre map instance | ||
| * @returns Array of control info | ||
| */ | ||
| export function getAllTrackedControls(map: Map): ControlInfo[] { | ||
| const registry = getControlRegistry(map); | ||
| return Object.values(registry); | ||
| } | ||
|
|
||
| /** | ||
| * Get serializable control info (without instance references) for state storage. | ||
| * | ||
| * @param map - MapLibre map instance | ||
| * @returns Array of serializable control info | ||
| */ | ||
| export function getSerializableControls(map: Map): SerializableControlInfo[] { | ||
| const registry = getControlRegistry(map); | ||
| return Object.values(registry).map(({ id, type, position, options }) => ({ | ||
| id, | ||
| type, | ||
| position, | ||
| options, | ||
| })); | ||
| } | ||
|
|
||
| /** | ||
| * Clear all tracked controls from the map. | ||
| * | ||
| * @param map - MapLibre map instance | ||
| */ | ||
| export function clearAllTrackedControls(map: Map): void { | ||
| const registry = getControlRegistry(map); | ||
|
|
||
| for (const controlInfo of Object.values(registry)) { | ||
| try { | ||
| map.removeControl(controlInfo.instance); | ||
| } catch { | ||
| // Control may already be removed | ||
| } | ||
| } | ||
|
|
||
| (map as unknown as Record<string, ControlRegistry>)[CONTROL_REGISTRY_KEY] = | ||
| {}; | ||
| } | ||
|
|
||
| /** | ||
| * Restore control registry from serialized state. | ||
| * Note: This only restores the registry metadata, not the actual controls. | ||
| * Use a controlFactory with setMapState to recreate controls. | ||
| * | ||
| * @param map - MapLibre map instance | ||
| * @param controls - Serializable control info array | ||
| * @param controlFactory - Factory function to recreate controls | ||
| */ | ||
| export function restoreControls( | ||
| map: Map, | ||
| controls: SerializableControlInfo[], | ||
| controlFactory: (info: SerializableControlInfo) => IControl | null | ||
| ): void { | ||
| // Clear existing tracked controls | ||
| clearAllTrackedControls(map); | ||
|
|
||
| const registry: ControlRegistry = {}; | ||
|
|
||
| for (const serializedInfo of controls) { | ||
| const control = controlFactory(serializedInfo); | ||
|
|
||
| if (control) { | ||
| // Add control to map | ||
| map.addControl(control, serializedInfo.position); | ||
|
|
||
| // Store in registry with the original ID | ||
| registry[serializedInfo.id] = { | ||
| id: serializedInfo.id, | ||
| type: serializedInfo.type, | ||
| position: serializedInfo.position, | ||
| options: serializedInfo.options, | ||
| instance: control, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| (map as unknown as Record<string, ControlRegistry>)[CONTROL_REGISTRY_KEY] = | ||
| registry; | ||
| } |
There was a problem hiding this comment.
This code lacks test coverage. The control tracking system (adding, removing, restoring controls) is a new feature that should have tests to verify the registry operations work correctly and controls are properly managed.
Comment on lines
+1
to
+3
| { | ||
| "type": "module" | ||
| } |
There was a problem hiding this comment.
The .vite/deps directory contains Vite-generated build artifacts that should not be committed to version control. These files are automatically generated during development and will differ between environments. Add .vite/ to .gitignore to prevent these files from being tracked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.