Leaflet 1.9 breaks Leaflet.heat

[Sergiobop]

Checklist

• I've looked at the documentation to make sure the behavior isn't documented and expected.
• I'm sure this is an issue with Leaflet, not with my app or other dependencies (Angular, Cordova, React, etc.).
• I've searched through the current issues to make sure this hasn't been reported yet.
• I agree to follow the Code of Conduct that this project adheres to.

Steps to reproduce

First of all, thanks for your work!

The steps to reproduce the issue is just updating leaflet to 1.9 and use the leaflet.heat package. (https://github.com/Leaflet/Leaflet.heat)
I also use the @types/leaflet and @types/leaflet-heat packages.

Expected behavior

This worked before 1.9.0:

import * as L from 'leaflet';
import 'leaflet.heat';
....

const heatmapLayer = L.heatLayer(data, { radius: 15, max: 10, minOpacity: 0.75 });

Current behavior

The app breaks and produces the following error:

export 'heatLayer' (imported as 'L') was not found in 'leaflet' (possible exports: Bounds, Browser, CRS, Canvas, Circle, CircleMarker, Class, Control, DivIcon, DivOverlay, DomEvent, DomUtil, Draggable, Evented, FeatureGroup, GeoJSON, GridLayer, Handler, Icon, ImageOverlay, LatLng, LatLngBounds, Layer, LayerGroup, LineUtil, Map, Marker, Mixin, Path, Point, PolyUtil, Polygon, Polyline, Popup, PosAnimation, Projection, Rectangle, Renderer, SVG, SVGOverlay, TileLayer, Tooltip, Transformation, Util, VideoOverlay, bind, bounds, canvas, circle, circleMarker, control, default, divIcon, extend, featureGroup, geoJSON, geoJson, gridLayer, icon, imageOverlay, latLng, latLngBounds, layerGroup, map, marker, noConflict, point, polygon, polyline, popup, rectangle, setOptions, stamp, svg, svgOverlay, tileLayer, tooltip, transformation, version, videoOverlay)

Minimal example reproducing the issue

No response

Environment

• Leaflet version: 1.9.0
• Browser (with version): All
• OS/Platform (with version): All

[Falke-Design]

@jonkoops can you look into this?

[jonkoops]

Sure, I'll take a look 👍

[jonkoops]

I've created this example project which reproduces the issue using Vite. The issue comes from the way Leaflet is imported:

import * as L from 'leaflet'

Since this imports all of the named exports from Leaflet, but not the global L (which is the default export) L.heatLayer can not be found.

I managed to fix this issue by importing the default export instead:

-import * as L from 'leaflet'
+import L from 'leaflet'

Alternatively, you can even omit naming the import as Leaflet is exposed globally.

import 'leaflet'
const map = L.map('map') // Still works

[jonkoops]

This is still a backwards compatibility issue however, although I don't think there is a way we can fix this elegantly. The only way I can see this working is by removing the module entry point. There is no way to augment the namespaced import from a module that imports it.

[jonkoops]

The way I see it we can resolve this two ways:

1. Add migration notes to our release for users that are using the namespaced import.
2. Remove the entrypoint to the ECMAScript module version from the package and release a patch.

I'm leaning towards the first, even though it's a breaking change for users of namespaced import. The reason for this is that it allows a migration path to the next major version, which will drop support for other module systems.

However, other core team members might feel differently. Especially if this ends up impacting too many users. @mourner @Malvoz @Falke-Design WDYT?

[mourner]

I'm leaning towards the first, even though it's a breaking change for users of namespaced import. The reason for this is that it allows a migration path to the next major version, which will drop support for other module systems.

Yeah, I think it makes sense. The plugins working with a namespaced import is already a hack that shouldn't have been working in the first place, and gives the illusion of proper ES with tree-shaking even though it doesn't tree-shake at all.

I'd move the ESM changelog item under "Breaking changes" and add a note about plugin compatibility. But I don't really know how much of a pain point this would be so let's wait for other opinions.

[Falke-Design]

I see a little bit the problem with unmaintained plugins. What if we publish to sources like before with .esm.js?

[jonkoops]

Ahh, as in plugins that might be using the namespaced import method already? Yeah, those could indeed exist, but I have no idea how many. A lot of existing 'legacy' plugins all seem to rely on the global L, which should remain compatible (even in 2.0).

[Falke-Design]

Is there a good way to search through Github for imports? I would do it but I'm currently working on the Event Hotfix and need to go in 40 minutes

[mourner]

Should we do a quick v1.9.1 with the listens fix before we decide on this one, and do a v1.9.2 later if necessary? Or should we wait?

[Falke-Design]

https://github.com/search?q=%22import+*+as+L+from+%27leaflet%27%3B%22&type=code

"import * as L from 'leaflet';" --> 12,945 code results 😶

[Falke-Design]

Should we do a quick v1.9.1 with the listens fix before we decide on this one, and do a v1.9.2 later if necessary? Or should we wait?

Yes I think that would be good.
But if you guys think you find a solution in the next hours, it is also ok for me to wait. I need to go now, but I think you can handle this without me 😄

[jonkoops]

Yeah, let's do a quick release before we decide what to do here. Sounds good to me.

[mourner]

v1.9.1 released.

[jonkoops]

@Sergiobop could you verify that the fixes I mentioned above also work for you? I'd like to make sure that it's not just 'works on my machine'.

[Sergiobop]

Hey @jonkoops

Option 1:
import L from 'leaflet'

My IDE and app complains:

Module '"..../node_modules/@types/leaflet/index"' has no default export. 

Option 2:
import 'leaflet';

My IDE and app complains when i use L.

TS2686: 'L' refers to a UMD global, but the current file is a module. Consider adding an import instead.

[jonkoops]

Thanks for checking @Sergiobop. So the first problem is likely issue due to the fact that @types/leaflet not yet being updated to reflect the changes in Leaflet.

You might be able to work around this by following the instructions in #8467 (comment).

[SamuelGaona]

Same with GeometryUtil from leaflet-draw

[Falke-Design]

@jonkoops @mourner what is the plan now? What should we do? I think it is not ok, to make that big breaking change in 1.9

[jonkoops]

We could revert this work in a patch release and then re-introduce it in 2.0. It looks like this is creating too much fallout.

[cyrilirudayaraj]

We could revert this work in a patch release and then re-introduce it in 2.0. It looks like this is creating too much fallout.

@jonkoops Can we expect the patch release sooner?

[jonkoops]

@cyrilirudayaraj we're bundling multiple issues that need a resolution (see the milestone).

[mourner]

@jonkoops do you want to do the v1.9.2 release some time soon?

[jonkoops]

Yeah, I'd like to do it ASAP. I don't have time to get to it today, so if someone wants to go ahead now feel free. Just make sure all the critical stuff is cherry-picked on v1 (which I think I already did).

[VashJuan]

Not suggesting a change of approach here, just noting other options that some of us have resorted to.

For me, to get 1.9.x working, I searched on the error and came up with workaround:
[old] myMarkerCluster = L.markerClusterGroup()
[new] myMarkerCluster = new window.L.MarkerClusterGroup()

Best explained at:
https://stackoverflow.com/a/71574063/18004414

Other possibly relevant discussions:
https://stackoverflow.com/questions/48960970/using-umd-globals-with-modules-in-a-typescript-project