modules, globals, and plugins

[sheppard]

It looks like 0.8 will not expose a global if a module system (i.e. Node or AMD) is present. This is definitely a good thing, but it will make it a bit trickier to integrate Leaflet plugins that don't have a UMD wrapper. Without the global, said plugins will not have anything to attach to (even when using e.g. a RequireJS shim config).

What do you think about recommending (but not requiring) that all Leaflet plugins have a UMD wrapper? Proj4leaflet and Leaflet.zoomslider could be good base examples.

The two concrete steps I have in mind are:

• Update the plugin authoring guide to (briefly) discuss the two main module systems and recommend adding a UMD wrapper for compatibility with both
• Update all of the "official" plugins (in @Leaflet) to have UMD wrappers

I am willing to take action on these as I have time, but thought it would be good to start by determining an overall strategy.

(I should note that I personally avoid compatibility issues by manually adding AMD wrappers to every third party library I work with that doesn't have one. Needless to say that isn't a very scalable approach.)

[mourner]

@sheppard hmm, good question... Maybe not exposing a global variable is not worth it if you have to enforce something on plugins that otherwise don't work. Besides, for people that don't want the global var, it's enough to call L.noConflict() to ditch it. So maybe revert the change, what do you think?

[sheppard]

I actually would suggest keeping the new structure and dealing with any issues that arise on a case-by-case basis. The global is just one of various issues that come from mixing modules with non-modular code, and the best long term fix is always to add module definitions to any files that don't have them. I guess it's not clear to me how many people:

1. are using Leaflet with plugins, and
2. are using a module loader instead of individual <script> tags, and
3. aren't familiar with how to add moduley stuff to JS files that don't have them

Only users who have all three characteristics should be affected by this issue. As long as there is a clear answer for any issues that come up things should be fine. I personally think standardizing on UMD is logical extension to the existing rule "Never expose global variables in your plugin." However, if you don't want to enforce UMD on plugin maintainers, we could suggest various workarounds for users in the above group:

• Work with the plugin maintainer to add a UMD definition
• Use a project management tool like volo that knows how to take non-modular javascript files and add module wrappers and dependencies
• Manually add module wrappers to vendored copies of plugins in your project (my current approach)
• Create an "adapter" module for Leaflet that restores the global. (Again, this does not necessarily address various other module+non-module issues, particularly related to how to get everything to load in the proper order)

// leaflet-global.js
define(['leaflet'], function(L) {
    window.L = L;
    return L;
});

[sheppard]

As a point of reference, d3 recently went directly from only exporting a global, to declaring a private module if an AMD loader is present. Initially, there were some issues by third party users (d3/d3#1693) but it seems they were sorted out relatively quickly.

[sheppard]

Here's a potential UMD wrapper for an arbitrary Leaflet plugin:

(function (factory) {
    if (typeof define === 'function' && define.amd) {
        // AMD
        define(['leaflet'], factory);
    } else if (typeof module !== 'undefined') {
        // Node/CommonJS
        module.exports = factory(require('leaflet'));
    } else {
        // Browser globals
        if (typeof this.L === 'undefined')
            throw 'Leaflet must be loaded first!';
        factory(this.L);
    }
}(function (L) {

L.MyPlugin = {
    /* ... */
};
return L.MyPlugin;

}));

[tmcw]

Tacking methods and classes onto the L namespace always seemed odd to me (hence quite a few of my plugins not doing that). I'd love to see something like a .use() style plugin system instead.

[sheppard]

I agree there wouldn't be a need to use the L namespace - particularly if a module system was in use. To modify the above example:

(function (factory) {
    if (typeof define === 'function' && define.amd) {
        // AMD
        define(['leaflet'], factory);
    } else if (typeof module !== 'undefined') {
        // Node/CommonJS
        module.exports = factory(require('leaflet'));
    } else {
        // Browser globals
        if (typeof this.L === 'undefined')
            throw 'Leaflet must be loaded first!';
        this.MyPlugin = factory(this.L);
    }
}(function (L) {

var MyPlugin = {
    /* ... */
};

return MyPlugin;
}));

The above would define a global variable for the plugin if no module system was present.

I suppose a use() style function would lessen the need for the UMD boilerplate on each individual plugin. Something like this?

// myplugin.js

// UMD or whatever author prefers - no dependency on Leaflet (?!).
define(function() {

// Plugin definition accepts Leaflet as an argument
function MyPlugin(L) {
    MyPlugin.doSomething = function() {
        L.doSomething();
    }
}
return MyPlugin;

});

// myapp.js

// Load deps using system of choice
define(['leaflet', 'myplugin'],
function(L, MyPlugin) {

// Register plugin with Leaflet (or rather, Leaflet with plugin)
L.use(MyPlugin);
MyPlugin.doSomething();

});

Not sure I did that right but you get the idea. It's a little different than the use() in slate-irc, which appears to register plugins with a function instance rather than a singleton variable. (Or are you suggesting that L could/should become a function returning Leaflet instances?)

[sheppard]

ICYMI: d3 has gone back to always exporting a global (d3/d3#1921)

[mourner]

I think I'll have to agree with Mike. We need to bring back the global.

[sheppard]

Fair enough, it seems like that might be the most practical solution for now.

[patrickarlt]

Global is back as of #2943. I've also updated the plugin guide with some best practices for plugin authors so plugins don't have to rely on the global if browserify or require js are present.

[Vanuan]

With ES modules being around the corner to be shipped to browsers, would it make sense to reconsider?

[jperelli]

ES modules has been shipped in major browsers as of September 15, 2017.
https://developers.google.com/web/updates/2017/09/nic61

[tmcw]

That's a significant overstatement: in Firefox & Edge, modules are still behind a flag, and on Android, there's no support. Global feature coverage is around 44%. Maybe sometime soon they'll be something you can count on, but they aren't right now.

[jperelli]

@tmcw Maybe I wasn't specific enough (non-native english), I didn't want to say ALL major browsers but SOME, although 44% seems a lot to me. Also take a count for react/vue user base that use babel.
My point was that this is happening, I know that it is not going to be used by all leaflet users right now, but I think it is important to be noted.
It was just an "update" comment to emphasize that module support is imminent.