aratcliffe / Leaflet.contextmenu

A context menu for Leaflet.
MIT License
367 stars 107 forks source link

Problem with Electron #116

Open heretic13 opened 6 years ago

heretic13 commented 6 years ago

Hi.

I create some applications with Electron. Now I have a problem with "Leaflet.contextmenu". I'ts not working. I began investigating and found a bug place:

(function(factory) { // Packaging/modules magic dance var L; if (typeof define === 'function' && define.amd) { // AMD define(['leaflet'], factory); } else if (typeof module === 'object' && typeof module.exports === 'object') { // Node/CommonJS L = require('leaflet'); module.exports = factory(L); } else { // Browser globals if (typeof window.L === 'undefined') { throw new Error('Leaflet must be loaded first'); } factory(window.L); } })

The fact is that Electron supports the require method on web pages(since it is based on Node.js). But this does not mean that it is necessary to load the leaflet module.

Here I see two nuances:

  1. If I wanted to write an Electron application, I could probably use the leaflet module (I have never done that and I don’t know if it will work) instead of explicitly registering the link to leaflet.js on my html page. But I think it is necessary to give the user a choice of how to act.
  2. A more interesting case is when Electron loads a web page from a site and the page uses leaflet and Leaflet.contextmenu. You cannot change the page content and should not force Electron to load the leaflet module. You must use a leaflet.js that is explicitly registered on the page.

I do not know the right decision.

heretic13 commented 6 years ago

Solution is simple:

comment this: / else if (typeof module === 'object' && typeof module.exports === 'object') { // Node/CommonJS L = require('leaflet'); module.exports = factory(L); } /

Please anybody make a pull request.

Some advices from me to community:

  1. leaflet as a module - very bad idea. It has limited scope - only browsers that support modules and agree it.
  2. autoload 'leaflet as a module' for leaflet.contextmenu - is a still a big bad idea. LOAD LEAFLET MANUALLY before load leaflet.contextmenu.js !!!!