Leaflet / Leaflet

πŸƒ JavaScript library for mobile-friendly interactive maps πŸ‡ΊπŸ‡¦
https://leafletjs.com
BSD 2-Clause "Simplified" License
40.17k stars 5.75k forks source link

Patch Leaflet to be Server-Side-Render friendly (SSR) #9316

Closed matthova closed 1 month ago

matthova commented 1 month ago

This Pull request adds the eslint plugin ssr-friendly and implements changes to meet the new linting requirements.

I've confirmed I'm able to use the forked library with a SSR Remix app.

IvanSanchez commented 1 month ago

And do read https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining .

matthova commented 1 month ago

@IvanSanchez thank you for the feedback! I love the idea of enforcing ssr-compatibility via the lint plugin I added, however, I wonder if there's a slightly different approach we could take for this library to remove unnecessary function-wrapping. Let me do some digging

jonkoops commented 1 month ago

I've confirmed I'm able to use the forked library with a SSR Remix app.

I am curious about this. Why would Leaflet need to be 'SSR friendly' in order for this to work? Leaflet was always intended to be a complete client-side solution, so why even use it in an SSR context? Can Remix not opt-out of SSR for the few places one might need to include Leaflet?

matthova commented 1 month ago

I've confirmed I'm able to use the forked library with a SSR Remix app.

I am curious about this. Why would Leaflet need to be 'SSR friendly' in order for this to work? Leaflet was always intended to be a complete client-side solution, so why even use it in an SSR context? Can Remix not opt-out of SSR for the few places one might need to include Leaflet?

That's a great question. I think the maintainers of the library need to own this decision. My use case, as a consumer, is that I love being able to install a package and not have to think about it or do any additional configuration. The tradeoff may not be worth it, for maintainers.

matthova commented 1 month ago

Closing and will submit an alternate approach with optional chaining

matthova commented 1 month ago

Alternate PR: https://github.com/Leaflet/Leaflet/pull/9317