Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

window.L not set if using a module system #5489

Closed
perliedman opened this issue May 2, 2017 · 3 comments
Closed

window.L not set if using a module system #5489

perliedman opened this issue May 2, 2017 · 3 comments
Assignees
Labels
blocker Critical issue or PR that must be resolved before the next release
Milestone

Comments

@perliedman
Copy link
Member

With the new Rollup build, the UMD loader will not set window.L if you're using a module system, like when building with Browserify.

This is a breaking change, and breaks a number of plugins, which rely on window.L being set instead of explicitly importing or requiring Leaflet.

Related discussion in #2364, where the conclusion was that 1.0 (or 0.8 as it was back then) should set window.L.

@perliedman perliedman added the blocker Critical issue or PR that must be resolved before the next release label May 2, 2017
@perliedman perliedman added this to the 1.1 milestone May 2, 2017
@ghybs
Copy link
Collaborator

ghybs commented May 3, 2017

Hi,

FWIW, many developers that I see asking questions (SO, GIS SE…) and using build tools (not so many people in the end, to be honest) indeed seem to expect window.L to be set.
E.g. see this post on SO, and even #4964.
And so do many plugins, as already mentioned.

Here is an interesting post that does propose a workaround to expose globally even through the UMD wrapper (it also cites the d3 example BTW).
Unfortunately I guess that would require making a Feature Request to Rollup?

As a side note, I think it could be interesting to provide a simple ES module entry file for power users actually using an ES module loader and expecting their build tool to optimize the bundle (I guess that it is what is implied in that comment).

Here is a quick example in SystemJS.
It basically uses an entry file almost like src/Leaflet.js, but without the version import from package.json (which would require a special plugin for JSON in SystemJS, if I understand correctly) nor the noConflict code.

And with a simple plugin wrapped with a new import and export wrapper, instead of the standard UMD.
(using Leaflet.FeatureGroup.SubGroup plugin, but could probably be applied to most plugins that have a build process)

As previously, even that example demonstrates that the biggest drawback is breaking plugins that do not adapt to the new pattern…

But at least this is just an extra option that is cheap to add (e.g. d3 index file).
And I feel this was one of the potential benefits of switching to Rollup?

@perliedman
Copy link
Member Author

@ghybs I read your post above in detail first now. I haven't read carefully enough to get all the details, but I think this sounds very good.

Is this something you think you could make a PR of, so that we have something more to discuss around going forward? It seems we need to come up with a plan on how to move forward with ES6 modules, plugins and bundlers.

@ghybs
Copy link
Collaborator

ghybs commented Jul 10, 2017

Hi,

Sure let me set up something simple.

I feel we could get a lot of inspiration from d3 build process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Critical issue or PR that must be resolved before the next release
Projects
None yet
Development

No branches or pull requests

3 participants