Export as an ES module

[bertday]

It would be helpful if this package exposed the measure control as an ES module for use with tools such as Rollup.

[matt-e-king]

@rbrtmrtn I didn't have any issues using this in a webpack environment and treating it as an ES6 module.

Have you tried importing Leaflet in the same module, like this:

import Leaflet from 'leaflet'
import 'leaflet-measure'

By doing so, leaflet-measure will look for the Leaflet object and attached itself to it like in the README:

Leaflet.Control.Measure

You can even be diligent and do something like this:

import Leaflet from 'leaflet'
import Measure from 'leaflet-measure'
import { isEmpty} from 'lodash'

// and when you need leaflet-measure, make sure it exists

// using lodash.isEmpty to simplify example
if (isEmpty(Leaflet.Control.Measure)) {
  Leaflet.Control.Measure = Measure
}

Not sure if that last example works 100%, but the idea is there.

[bertday]

Thanks @matt-e-king, I didn't try importing it that way but I'll give it a shot. Somehow the module way feels a little more future-proof, but I'm fine with whatever works :)

[bertday]

Hi @matt-e-king, question about this -- it looks like the code in src/leaflet-measure.js patches the L global without importing it. Is there some way to configure webpack so it knows how to find L?

[matt-e-king]

Hey @rbrtmrtn, that is a little bit of a loaded question. I'm not as well versed on webpack/babel as I'd like to be either, but I'll break this into two sections with my best educated guess:

it looks like the code in src/leaflet-measure.js patches the L global without importing it. At it's core, babel-loader is just transpiling "Javascript" into "backwards compatible Javascript". It is not going to catch everything that would normally trigger an exception, e.g. L is not defined. What will throw an exception is if you run this transpiled code through some sort of runtime environment, like webpack-dev-server. Then it would throw an error because L is not defined.

The author of this leaflet-measure plugin designed this in a way that we, the developers, who are ingesting the plugin are responsible to provide the global L object.

That is why you don't see an import for L inside of src/leaflet-measure.js

With that said... Is there some way to configure webpack so it knows how to find L? This answer is now under a slightly different context. If you as the developer are trying to use leaflet-measure within a module environment, you will need to import Leaflet. Or at least provide it at a global level outside of your webpack build, BUT webpack will need to be aware of that scenario.

Try looking into the following things: https://webpack.js.org/configuration/externals/ https://webpack.js.org/guides/shimming/

[brandoncopeland]

@matt-e-king has the right idea.

The plugin guidelines for Leaflet suggest appending any new classes to the L namespace https://github.com/Leaflet/Leaflet/blob/master/PLUGIN-GUIDE.md#plugin-api and not packaging up Leaflet in the plugin source. This is reasonable, I think, because Leaflet plugins don't really make sense without the existence of Leaflet.

To use this or any other plugins following this pattern, you will first need to import Leaflet, load Leaflet from a CDN, or some other means of globally creating L.Control as @matt-e-king suggested.

[bertday]

Thanks @matt-e-king and @brandoncopeland for shedding light on this.

I feel like I might be missing something. This is what I tried doing:

import leaflet from 'leaflet';
import 'leaflet-measure';

and I get an error that L is not defined. That makes sense, because I'm not doing anything that would expose the L global -- I'm importing it as leaflet. However, as I mentioned earlier, there's a reference to L hardcoded in leaflet-measure.

The approach of appending to L (i.e. L.Control.Measure) makes sense to me for a browser build, but I'm not grasping how it would work via imports. It seems like there is a hard requirement to have a copy of Leaflet available as L in the global namespace, which seems like something of an anti-pattern (again, unless you're just bringing this in over CDN, in which case it's convenient and not an issue).

Would it make sense to have two webpack targets -- one for browsers, and another for ES module use?

[brandoncopeland]

I see. We could probably update to something like the following to resolve.

import { Control } from 'leaflet';
Control.Measure = ...

[bertday]

@brandoncopeland that seems like the right direction, but I think you might run into similar issues patching Control. I would suggest something like:

import { Control } from 'leaflet';

export default class MeasureControl extends Control {
  // override things here
}

[brandoncopeland]

Feel free to submit a PR if you like. If not, I'll try to get to it when I can. The ideal solution would continue to support users relying on global L.

[matt-e-king]

@rbrtmrtn I can see now why my original example might be a bit misleading. I wasn't too familiar with the differences between webpack and Rollup, but it sounds like out of the box, Rollup isn't compatible with CommonJS modules. (Please correct me if I'm wrong). But there are plugins for it.

If you look at the top of the leaflet main distribution file, it looks something like this:

/*
 Leaflet 1.0.3, a JS library for interactive maps. http://leafletjs.com
 (c) 2010-2016 Vladimir Agafonkin, (c) 2010-2011 CloudMade
*/
(function (window, document, undefined) {
var L = {
    version: "1.0.3"
};

function expose() {
    var oldL = window.L;

    L.noConflict = function () {
        window.L = oldL;
        return this;
    };

    window.L = L;
}

// define Leaflet for Node module pattern loaders, including Browserify
if (typeof module === 'object' && typeof module.exports === 'object') {
    module.exports = L;

// define Leaflet as an AMD module
} else if (typeof define === 'function' && define.amd) {
    define(L);
}

// define Leaflet as a global L variable, saving the original L to restore later if needed
if (typeof window !== 'undefined') {
    expose();
}

So you can see Leaflet is exposing a window.L, but for Node module patterns (i.e. CommonJS which is used by webpack) it also uses module.exports = L. Since L is a complex type, window.L and the module.export are going to be the same object in memory. Webpack/babel have no problem compiling it and bundling, and by the time I actually need it in runtime, window is available.

I'm fairly certain I got this to work through ignorance :)

(typeof module === 'object' && typeof module.exports === 'object') is probably false when Rollup transpiles it.

@rbrtmrtn at what point do you actually get the L is not defined exception?

[vinayakkulkarni]

would really appreciate if there was a PR / update for this.