sadpandajoe commented on code in PR #42608:
URL: https://github.com/apache/superset/pull/42608#discussion_r3830102408


##########
superset-frontend/plugins/plugin-chart-point-cluster-map/package.json:
##########
@@ -31,10 +31,14 @@
   "dependencies": {
     "@math.gl/web-mercator": "^4.1.0",
     "mapbox-gl": "^3.27.0",
-    "maplibre-gl": "^5.24.0",
+    "maplibre-gl": "^6.0.0",

Review Comment:
   MapLibre 6's Webpack integration requires the application to call  with an 
emitted  URL. This plugin renders the MapLibre adapter without a worker URL, 
and Superset's Webpack config does not emit that worker, so point-cluster maps 
can fail when the v6 worker starts. Could this configure and bundle the worker 
before upgrading this plugin to v6?



##########
superset-frontend/plugins/preset-chart-deckgl/src/mapLibreVersionPin.test.ts:
##########
@@ -0,0 +1,36 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * @deck.gl/mapbox's terrain-camera path (used by DeckGLOverlayMapLibre) reads
+ * maplibre-gl's internal `map.transform` property directly. maplibre-gl 6
+ * removed that property, so this package's maplibre-gl range stays pinned
+ * below 6 (see package.json) independent of the rest of the monorepo, which
+ * moved to maplibre-gl 6 in #42608.
+ *
+ * This is a tripwire, not a feature test: it fails loudly if the pin is ever
+ * lifted (package.json's maplibre-gl range bumped past ^5) without first
+ * confirming @deck.gl/mapbox no longer needs the removed property.
+ */
+test('maplibre-gl stays pinned below 6 until @deck.gl/mapbox drops its 
map.transform usage', () => {
+  // eslint-disable-next-line global-require
+  const { version } = require('maplibre-gl/package.json');

Review Comment:
   This resolves the preset-local MapLibre 5 copy, but  uses the root-hoisted , 
whose MapLibre peer resolves to the root v6 copy. A terrain-backed deck.gl 
chart can therefore still pass MapLibre 6 to  and hit its removed  access while 
this tripwire passes. Could the pin apply to the adapter's runtime dependency 
as well, or could this test assert the version that adapter resolves?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to