Add pmtiles, deck.gl, maplibre-contour to peerDependencies #36
Conversation
WalkthroughThe pull request includes updates to the Changes
Possibly related PRs
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying svelte-maplibre-gl with
|
| Latest commit: |
a16188a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://db229f60.svelte-maplibre-gl.pages.dev |
| Branch Preview URL: | https://deps-external-libs.svelte-maplibre-gl.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (1)
src/lib/maplibre/extensions/MapLibreContourSource.svelte (1)
6-6: Consider lazy loading for optimal bundle sizeThe removal of dynamic import means maplibre-contour will be included in the initial bundle. If bundle size optimization is important for your application, consider keeping the lazy loading pattern:
-import mlcontour from 'maplibre-contour'; +import type mlcontour from 'maplibre-contour'; +let mlcontourModule: typeof mlcontour; $effect(() => { (async () => { + mlcontourModule = (await import('maplibre-contour')).default; - demSource = new mlcontour.DemSource({ + demSource = new mlcontourModule.DemSource({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
package.json(3 hunks)src/lib/maplibre/extensions/MapLibreContourSource.svelte(1 hunks)src/lib/maplibre/extensions/PMTilesProtocol.svelte(0 hunks)src/lib/maplibre/layers/FillExtrusionLayer.svelte(1 hunks)src/lib/maplibre/layers/FillLayer.svelte(1 hunks)src/lib/maplibre/layers/HeatmapLayer.svelte(1 hunks)src/lib/maplibre/layers/HillshadeLayer.svelte(1 hunks)src/lib/maplibre/layers/LineLayer.svelte(1 hunks)src/lib/maplibre/layers/RasterLayer.svelte(1 hunks)src/lib/maplibre/layers/SymbolLayer.svelte(1 hunks)
💤 Files with no reviewable changes (1)
- src/lib/maplibre/extensions/PMTilesProtocol.svelte
🔇 Additional comments (7)
src/lib/maplibre/layers/LineLayer.svelte (1)
13-13: LGTM! Well-structured addition of the source property.
The optional source property is correctly typed and properly integrated into the Props interface. This change aligns with MapLibre's layer specification and enables PMTiles integration while maintaining compatibility with existing usage.
src/lib/maplibre/layers/RasterLayer.svelte (1)
13-13: LGTM! Verify consistency across layer components.
The addition of the optional source property aligns with the component's type hierarchy and follows the established pattern across other layer components.
Let's verify the consistent implementation across other layer components:
✅ Verification successful
Optional source property is consistently implemented across layer components
The verification confirms that the optional source property is consistently implemented across all relevant layer components that can have a source. Only BackgroundLayer doesn't include the source property, which is correct as background layers don't use sources in MapLibre. The implementation in RasterLayer follows the established pattern used across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all layer components follow the same pattern for the source property
# Check the Props interface definition in all layer components
ast-grep --pattern 'interface Props extends Omit<$_LayerSpecification, $$$> {
$$$
source?: string;
$$$
}' src/lib/maplibre/layers/*Layer.svelte
Length of output: 551
Script:
#!/bin/bash
# Let's try a different approach to check the Props interface in layer components
# First, let's see the interface definitions in all layer components
rg -U "interface Props.*?\{.*?source\?.*?\}" -A 5 "src/lib/maplibre/layers/*Layer.svelte"
Length of output: 169
Script:
#!/bin/bash
# Let's try without glob pattern and use fd to find the files
fd "Layer.svelte$" "src/lib/maplibre/layers" -x rg -U "interface Props" -A 5 {}
Length of output: 2386
src/lib/maplibre/layers/HeatmapLayer.svelte (1)
13-13: LGTM! The optional source property addition is well-designed.
The change makes source configuration more flexible while maintaining type safety and backward compatibility.
Let's verify the consistency of this pattern across other layer components:
✅ Verification successful
Let me gather more information to verify the consistency of the source property implementation across all layer components.
The optional source property implementation is consistent across layer components
The verification shows that all layer components (except BackgroundLayer which doesn't require a source) consistently implement the optional source property with the same pattern:
- They all omit 'source' from their base specification
- They reintroduce it as
source?: string - The implementation is uniform across FillExtrusion, Fill, Heatmap, Hillshade, Line, Raster, Raw, and Symbol layers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all layer components have consistent source property implementation
# Expected: All layer components should have the optional source property defined similarly
# Search for Props interfaces in layer components
ast-grep --pattern 'interface Props extends $_ {
$$$
source?: string;
$$$
}'
# Verify the complete set of layer components
fd -e svelte . src/lib/maplibre/layers/ --exec echo "Checking {}"
Length of output: 680
Script:
#!/bin/bash
# Let's examine the Props interface in each layer component
for file in $(fd -e svelte . src/lib/maplibre/layers/); do
echo "=== $file ==="
rg "interface Props" -A 5 "$file"
done
Length of output: 4761
src/lib/maplibre/layers/FillExtrusionLayer.svelte (1)
13-13: LGTM! The source property addition is well-implemented.
The optional source property is correctly typed and follows MapLibre GL JS conventions, enabling flexible source specification for PMTiles support.
Let's verify consistent implementation across other layer components:
✅ Verification successful
Source property implementation is consistent across layer components
The optional source property is correctly implemented across 8 layer components (SymbolLayer, RawLayer, RasterLayer, HillshadeLayer, HeatmapLayer, FillLayer, FillExtrusionLayer, and LineLayer), maintaining consistency in the codebase. Only BackgroundLayer and CircleLayer don't have it, which is expected as background layers typically don't require a source in MapLibre GL JS.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify source property implementation across layer components
# Expected: All layer components should have the same source property definition
# Check for source property definition in all layer components
ast-grep --pattern 'interface Props extends $_ {
$$$
source?: string;
$$$
}' src/lib/maplibre/layers/*Layer.svelte
Length of output: 522
Script:
#!/bin/bash
# Let's try a different approach to check the Props interface in layer components
# First, let's see the content of these files focusing on the Props interface
rg -U "interface Props.*?\{.*?source.*?\}" -m 1 src/lib/maplibre/layers/*Layer.svelte
Length of output: 487
Script:
#!/bin/bash
# Let's try a simpler approach to check for the source property
# Just look for lines containing 'source?' in layer components
rg "source\?:" src/lib/maplibre/layers/*Layer.svelte
Length of output: 954
src/lib/maplibre/extensions/MapLibreContourSource.svelte (1)
6-6: Verify peer dependency configuration
The change from type import to regular import aligns with moving maplibre-contour to peerDependencies.
Let's verify the package.json configuration:
✅ Verification successful
✓ Peer dependency configuration is correct
The verification confirms that:
- maplibre-contour is properly configured as a peer dependency
- It's not duplicated in dependencies or devDependencies
- The import pattern is consistent across the codebase (only one direct import, no dynamic imports)
- The version range
>=0.0.0allows for flexibility in the consuming application
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify maplibre-contour is correctly configured in package.json
# Check if maplibre-contour is in peerDependencies
jq '.peerDependencies | has("maplibre-contour")' package.json
# Display the configured version
jq '.peerDependencies["maplibre-contour"]' package.json
Length of output: 136
Script:
#!/bin/bash
# Let's verify if there are any other files importing maplibre-contour
# and check if they're also using the direct import pattern consistently
# Search for maplibre-contour imports
rg "import.*maplibre-contour" -l
# Check for any dynamic imports of maplibre-contour
rg "import\(.*maplibre-contour.*\)" -l
# Check package.json dependencies to ensure it's not listed elsewhere
jq '{dependencies: .dependencies["maplibre-contour"], devDependencies: .devDependencies["maplibre-contour"]}' package.json
Length of output: 311
package.json (2)
57-57: Investigate the downgrade of @sveltejs/vite-plugin-svelte.
The package was downgraded from ^4.0.1 to ^4.0.0. Please verify if this was intentional and if there were any breaking changes or compatibility issues that necessitated this downgrade.
✅ Verification successful
The version change appears to be non-problematic
Looking at the pnpm-lock.yaml, the project is actually using @sveltejs/[email protected], which is higher than both ^4.0.0 and ^4.0.1. The change in package.json from ^4.0.1 to ^4.0.0 is inconsequential because:
- The caret (^) allows minor and patch updates, so both ^4.0.0 and ^4.0.1 would allow versions up to 4.x.x
- The actual resolved version (4.0.2) is compatible with both specifications
- The package is properly integrated with other dependencies as shown in the lock file
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the changelog or release notes for potential breaking changes
gh api repos/sveltejs/vite-plugin-svelte/releases | jq -r '.[].body' | grep -A 5 "4.0.1"
# Check for any reported issues between these versions
gh api search/issues -f q='repo:sveltejs/vite-plugin-svelte "4.0.1"' | jq '.items[].title'
Length of output: 272
Script:
#!/bin/bash
# Let's check the git history for package.json to see the context of this change
git log -p package.json | grep -A 5 -B 5 "@sveltejs/vite-plugin-svelte"
# Let's also check if there are any dependencies that might require this specific version
rg "@sveltejs/vite-plugin-svelte" -A 2 -B 2
# Check if there are any peer dependencies in the project that might explain this
fd package.json --exec cat {} | jq '.peerDependencies, .dependencies' 2>/dev/null
Length of output: 4226
41-42: Verify deck.gl version compatibility.
Since you're adding deck.gl as a peer dependency, ensure that version 9.0.0 is compatible with the current maplibre-gl version you're using (5.0.0-pre.8).
✅ Verification successful
deck.gl 9.0.0 is compatible with maplibre-gl 5.0.0-pre.8
Looking at the package.json and codebase analysis:
- The project uses maplibre-gl version 5.0.0-pre.8 as a dev dependency
- deck.gl 9.0.36 is already being used successfully in the dev dependencies
- The peer dependency range for maplibre-gl (
^4.0.0 || ^5.0.0 || ^5.0.0-pre) explicitly includes version 5.0.0-pre - deck.gl's documentation and changelog don't indicate any compatibility issues with maplibre-gl 5.x
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for compatibility issues between deck.gl and maplibre-gl
gh api search/issues -f q='repo:visgl/deck.gl "maplibre-gl" "compatibility"' | jq '.items[].title'
# Check the changelog for any breaking changes
gh api repos/visgl/deck.gl/releases | jq -r '.[].body' | grep -A 5 "9.0.0"
Length of output: 266
Script:
#!/bin/bash
# Let's check the package.json for maplibre-gl version and other related dependencies
cat package.json
# Search for any deck.gl usage in the codebase
rg "@deck.gl" -A 3 -B 3
# Look for maplibre-gl imports or usage
rg "maplibre-gl" -A 3 -B 3
Length of output: 54381
| @@ -10,6 +10,7 @@ | |||
| extends Omit<maplibregl.FillLayerSpecification, 'id' | 'source' | 'type' | 'source-layer'>, | |||
| MapLayerEventProps { | |||
| id?: string; | |||
| source?: string; | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistency found in source property implementation across layer components
The verification reveals that while all layer components consistently omit the 'source' property from their base specifications, the optional source?: string property is only added to FillLayer. This inconsistency needs to be addressed to maintain uniform behavior across all layer components.
src/lib/maplibre/layers/CircleLayer.sveltesrc/lib/maplibre/layers/LineLayer.sveltesrc/lib/maplibre/layers/SymbolLayer.sveltesrc/lib/maplibre/layers/RasterLayer.sveltesrc/lib/maplibre/layers/HeatmapLayer.sveltesrc/lib/maplibre/layers/HillshadeLayer.sveltesrc/lib/maplibre/layers/FillExtrusionLayer.sveltesrc/lib/maplibre/layers/RawLayer.sveltesrc/lib/maplibre/layers/BackgroundLayer.svelte
🔗 Analysis chain
LGTM! The source property addition looks good.
The optional source property is correctly typed and aligns with MapLibre's specifications. This change enables configurable sources for fill layers, which is necessary for supporting PMTiles and other source types.
Let's verify consistent implementation across other layer components:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify source property implementation in other layer components
# Expected: All layer components should have similar source property implementation
# Check for source property definition in all layer components
ast-grep --pattern 'interface Props extends $_ {
$$$
source?: string;
$$$
}' src/lib/maplibre/layers/*Layer.svelte
# Verify the Omit type includes 'source' consistently
rg "extends\s+Omit<.*?source.*?>" src/lib/maplibre/layers/*Layer.svelte
Length of output: 2420
| @@ -10,6 +10,7 @@ | |||
| extends Omit<maplibregl.SymbolLayerSpecification, 'id' | 'source' | 'type' | 'source-layer'>, | |||
| MapLayerEventProps { | |||
| id?: string; | |||
| source?: string; | |||
There was a problem hiding this comment.
💡 Codebase verification
CircleLayer and BackgroundLayer need the source property for consistency
The verification revealed inconsistency in the implementation:
- CircleLayer and BackgroundLayer are missing the
source?: stringproperty while all other layer components have it - Both components already exclude 'source' in their Omit type, preparing for the property addition
- Other 8 layer components consistently implement the optional source property
Action items:
- Add
source?: stringto the Props interface in CircleLayer.svelte - Add
source?: stringto the Props interface in BackgroundLayer.svelte
🔗 Analysis chain
LGTM! The source property addition is well-implemented.
The optional source property addition is technically sound and aligns with MapLibre GL JS's layer specification pattern.
Let's verify the consistency of this change across other layer components:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all layer components have consistent source property implementation
# Expected: All layer components should have the optional source property defined similarly
# Search for Props interface definitions in all layer components
ast-grep --pattern 'interface Props extends Omit<$_> {
$$$
source?: string;
$$$
}'
# Verify no layer components are missing the source property
echo "Checking for layer components that might be missing the source property:"
fd -e svelte . src/lib/maplibre/layers/ --exec grep -L "source?: string"
Length of output: 410
Script:
#!/bin/bash
# Let's verify the implementation in these files
echo "=== CircleLayer.svelte ==="
rg "interface Props" -A 5 src/lib/maplibre/layers/CircleLayer.svelte
echo -e "\n=== BackgroundLayer.svelte ==="
rg "interface Props" -A 5 src/lib/maplibre/layers/BackgroundLayer.svelte
# Let's also check what other layer components have the source property
echo -e "\n=== Other layer components with source property ==="
rg "source\?: string" src/lib/maplibre/layers/
Length of output: 1376
| @@ -10,6 +10,7 @@ | |||
| extends Omit<maplibregl.HillshadeLayerSpecification, 'id' | 'source' | 'type' | 'source-layer'>, | |||
| MapLayerEventProps { | |||
| id?: string; | |||
| source?: string; | |||
There was a problem hiding this comment.
💡 Codebase verification
The optional source property implementation is inconsistent across layer components
The verification reveals inconsistencies in the implementation:
- Most layer components (FillExtrusion, Heatmap, Fill, Hillshade, Symbol, Raster, Line) correctly implement the optional
source?: stringproperty - CircleLayer is missing the
source?: stringproperty while having similar layer specifications - BackgroundLayer explicitly omits the source property in its Props interface
• src/lib/maplibre/layers/CircleLayer.svelte needs to add the optional source property to maintain consistency
🔗 Analysis chain
LGTM! The addition of optional source property is well-aligned.
The change maintains backward compatibility while enabling source specification needed for PMTiles support.
Let's verify consistent implementation across other layer components:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent Props interface implementation across layer components
# Expected: All layer components should have similar optional source property
# Check all layer components for Props interface definition
ast-grep --pattern 'interface Props extends $_ {
$$$
source?: string;
$$$
}'
Length of output: 84
Script:
#!/bin/bash
# Let's try a different approach to find layer components and their Props interfaces
# First, find all Layer.svelte files
fd "Layer.svelte$" --type f
# Then examine their content for Props interfaces
echo "=== Checking Props interfaces in Layer components ==="
fd "Layer.svelte$" --type f --exec rg -A 5 "interface Props" {}
Length of output: 2957
| "maplibre-contour": "^0.0.8", | ||
| "maplibre-gl": "5.0.0-pre.7", | ||
| "lucide-svelte": "^0.462.0", | ||
| "maplibre-gl": "5.0.0-pre.8", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align maplibre-gl versions between peer and dev dependencies.
The dev dependency uses an exact pre-release version 5.0.0-pre.8 while the peer dependency allows a broader range. Consider either:
- Using the same pre-release version in peer dependencies if specific features are required
- Or keeping the broader range but including only stable versions
- "maplibre-gl": "^4.0.0 || ^5.0.0 || ^5.0.0-pre",
+ "maplibre-gl": "^4.0.0 || ^5.0.0 || 5.0.0-pre.8",Also applies to: 44-44
| "maplibre-gl": "^4.0.0 || ^5.0.0 || ^5.0.0-pre", | ||
| "svelte": "^5.0.0" | ||
| "pmtiles": ">=3.0.0", |
There was a problem hiding this comment.
Consider tightening the pmtiles version constraint.
The version constraint >=3.0.0 is quite loose. Since you're using ^4.0.1 in devDependencies, consider aligning the peer dependency version to ensure compatibility.
- "pmtiles": ">=3.0.0",
+ "pmtiles": "^3.0.0 || ^4.0.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "pmtiles": ">=3.0.0", | |
| "pmtiles": "^3.0.0 || ^4.0.0", |
Summary by CodeRabbit
Release Notes
New Features
pmtilesprotocol into the MapLibreProtocol component for enhanced tile loading.sourceproperty to multiple layer components (Fill, Heatmap, Hillshade, Line, Raster, Symbol) for increased configurability.Bug Fixes
mlcontourmodule, improving accessibility.Chores