search

esm7 / obsidian-map-view

Interactive map view for Obsidian.md
MIT License
604 stars 31 forks source link

Request - Please add leaflet-fullscreen module #6

Closed valentine195 closed 3 years ago

valentine195 commented 3 years ago

Hi,

I use leaflet-fullscreen to enable a fullscreen button in Obsidian Leaflet, but it seems if your plugin is loaded after Obsidian Leaflet, fullscreen plugin is removed (probably because it re-imports the leaflet library), which breaks my plugin unfortunately.

Could you please include that package in your plugin?

Thanks.

esm7 commented 3 years ago

Oh no, will get it fixed as soon as I manage! Regardless, perhaps we should open a bug for Obsidian's API, plugins should not be able to step on each other like that :(

On Thu, Jul 8, 2021 at 7:28 PM Jeremy Valentine @.***> wrote:

Hi,

I use (leaflet-fullscreen)[ https://www.npmjs.com/package/leaflet-fullscreen] to enable a fullscreen button in Obsidian Leaflet, but it seems if your plugin is loaded after Obsidian Leaflet, fullscreen plugin is removed (probably because it re-imports the leaflet library), which breaks my plugin unfortunately.

Could you please include that package in your plugin?

Thanks.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/esm7/obsidian-map-view/issues/6, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACRZ2YBFGZJIDFCJM3XEFQ3TWXG2PANCNFSM5ABCU3ZA .

valentine195 commented 3 years ago

I agree, but I think this is actually an issue with rollup?

I don't completely understand how bundlers work, but if you look at the generated main.js code, both of them are importing Leaflet - not sure Obsidian would be able to sandbox them.

Also, just wanted to let you know, I override quite a bit of Leaflet's CSS in my plugin. I'll be going through and making the selectors more specific, so that it does not target your maps, but if you see something that does not look like it should...it's probably from me :)

esm7 commented 3 years ago

Thanks, and I'll also see if there's a way to sandbox the Leaflet imports on my own. Though things like styles in the document header are probably going to be a challenge :-/

esm7 commented 3 years ago

Turns out it goes both ways :-/ For me obsidian-leaflet is loaded 2nd and it breaks a package I was using (Leaflet-ExtraMarkers). There must be a better solution than for us to coordinate all the Leaflet plugins that we use... Just not yet quite sure what it is.

valentine195 commented 3 years ago

Yeah, I saw your post on the Discord. Hopefully someone can help!

I wonder - could be possible to do a Dynamic Import instead of a static one to prevent run time compile collisions.

esm7 commented 3 years ago

So, good news first, I'm happy to announce that I solved the issue (or more correctly patched around it). As far as I could tell it works for both the scenario that obsidian-leaflet is loaded before Map View and after it (these pose two different problems, I tested by changing the order in community-plugins.json).

What didn't work: bundling Leaflet better (as was suggested in Discord) didn't help because the core issue seems to be the way Leaflet plugins use the global L namespace that Leaflet defines.

Fixing the case Map View is loaded first: The issue in this case was a single Leaflet plugin that was accessing L. I restructured the code so this plugin is accessed from just one place, and saved a copy of L after loading it. Then, every time that plugin is accessed, I replace the global L with the saved copy and then turn it back so obsidian-leaflet will keep having the copy it expects. Ain't pretty at all but that's the only solution I could find...

However, if obsidian-leaflet is loaded first, Map View overwrites L, and obsidian-leaflet needs that for the full-screen plugin. So I added the leaflet-fullscreen, imported it and also had to use it in a dummy variable so it won't get tree-shaked by Rollup :-/ @valentine195, if you can apply the same trick to make the full-screen plugin get its "expected" L every time it's used -- this will be great. Or possibly find a better solution than my ugly hack :-/

valentine195 commented 3 years ago

Ok, I believe I solved it as well.

Edit: Nevermind, I thought disabling and re-enabling the plugins would re-load the global L, but it seems it does not.

Edit 2: Okay, Obsidian Leaflet 3.19.5 seems to fix the collisions. Re-ordered and re-loaded Obsidian quite a few times with no errors from either plugin.