Support SSR

[matthova]

This PR is an alternative to https://github.com/Leaflet/Leaflet/pull/9316 It doesn't enforce nullish DOM access via lint, but, it does work 😅👍

[matthova]

@IvanSanchez Unfortunately optional chaining does not work if the window is undefined. The current pattern I'm using is to check via typeof

const pointer = typeof window === 'undefined' ? false : !!window.PointerEvent;

[jonkoops]

To get back to the discussion under #9316.

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.

Whilst I understand this concern and we want to make using Leaflet as user-friendly as possible, I am left wondering what the implications of this are? I am not sure about adding support for SSR when Leaflet is very much not made to ever be used in a server-side context.

What will this PR accomplish exactly? Will this cause Leaflet code be executed on the server? If so, this would yield additional overhead of code execution that does essentially nothing. I cannot imagine this will actually get as far as to render the map on the server and re-hydrate it on the client. In which case I might even consider it a feature that Leaflet doesn't work in such a context.

Could you provide a minimal example of how Leaflet would be integrated in a Remix application, what the roadblocks are you are running into and how this will resolve said problem without introducing unnecessary overhead?

[matthova]

@jonkoops I have a lot of thoughts about Remix, but the bigger picture is that this is a problem that would need a unique solution across all of the current fad of server-rendered UI frameworks, NextJS, SvelteKit, Remix, Nuxt, etc... (I swear some of these framework names are made up 😅 )

In all of these style of frameworks, there is a server that does a first pass of the UI, and because the server doesn't have global DOM variables, SSR support puts the onus on library devs to handle nullish global dom variables. It's not ideal, but SSR support removes a big painpoint for people using libraries in these scenarios.

I take the mental load for maintainers seriously, so I understand wanting to avoid any unnecessary features. Looking at the diff in this PR, I'm cautiously convinced this is minimal lift and minimal compute for a high impact use-case. That said, the final call falls on the library devs. Thank you for the work you do 🙏🏻

[Falke-Design]

Looking at the diff in this PR, I'm cautiously convinced this is minimal lift and minimal compute for a high impact use-case.

I see it like this too. Over time we got a few request for ssr support and if this few changes are all that are needed, then I would add them. But I would still not publish that we support ssr. It are just a few adjustments that makes it a little bit easier without offical support.

Other SSR references:

• 7085

• 6332

• 5402

[jonkoops]

I am still not convinced we should be landing this at all. My questions still remains, why execute Leaflet code on the server when it cannot actually do anything useful? What are the problems that will occur if we don't land this change?

From what I am seeing here we're simply introducing more variations in code branching, and taking on supporting various server-side run-times while we're at it. Which implicates that we now need to take this on as additional maintenance should these run-times have different interpretations or expediencies of how the code should be run.

For example, Deno has a window object, so the guard code written in this PR will not work. And who knows what other run-times will come up with?

I have a lot of thoughts about Remix, but the bigger picture is that this is a problem that would need a unique solution across all of the current fad of server-rendered UI frameworks, NextJS, SvelteKit, Remix, Nuxt, etc...

I understand that different SSR frameworks have different ways of dealing with client-side only code, but they all have escape hatches that allow users to run code on the client only (see Next.js, Remix and SvelteKit documentation). If a user is committing to use a framework, I think it's a valid expectancy they learn how to use it.

[mourner]

@jonkoops I'm not familiar with various nuances of SSR frameworks, but we had a similar situation in Mapbox GL JS — tons of requests from users to make it possible to load the bundle in a server-side environment for who knows what. Eventually we gave up and merged a similar PR and added a test which simply checked that node mapbox-gl.js doesn't throw any errors, and there were no issues or any kind of maintenance burden since.

Requiring Leaflet in some server script might not be a use case that makes a lot of sense for us (without much experience in this kind of SSR projects), but people will keep requesting it, and IMO it's a small price to pay.

why execute Leaflet code on the server when it cannot actually do anything useful?

Even if we don't consider actually rendering something on the server, they could at least use some utility functions Leaflet provides without resorting to another tool like Turf — geometric / geographical calculations, projections, etc.

[jonkoops]

Very well, if we have to land this I'll give this a proper pass. Let me take a look and put in a review.

[IvanSanchez]

I am still not convinced we should be landing this at all. My questions still remains, why execute Leaflet code on the server when it cannot actually do anything useful? What are the problems that will occur if we don't land this change?

@jonkoops I agree with the sentiment. All that this will do is waste some RAM in JS web servers, fail some checks, and produce nothing.

On the other hand, I think of this the same way I think of old MSIE compatibility code: we don't need it for the "normal" use case, it shouldn't be there, it's ugly but, but makes things work in some minority scenario.

[mourner]

There are still some globals referred to that might cause issues in a server-side run-time, but they will only create issues if DOM related APIs are used on the server, or if a Map is initialized.

In summary, I think we shouldn't anticipate any possible needs on this front by doing premature changes — instead let's accept the most minimal changes possible, as they look harmless and should cover immediate bases. Users who use Leaflet in SSR contexts can always follow-up with small fixes later, which can be reviewed separately.

[jonkoops]

Ok, if the issues that would occur under Deno SSR are fixed this can be merged as far as I am concerned.

[Gugustinette]

Thanks for considering these changes ! 🥹 This is also a common discussion in packages such as Nuxt Leaflet which I work on.

We do not exactly want to run Leaflet on the server, but just having it's instance loaded without throwing errors, so SSR environments can play with it (and most of the time, send it to the client on demand). This will make Leaflet very easy to use in these magic and modern framework.

I can try a kind of canary version of Leaflet on my module before merging if you want.

[NilsBaumgartner1994]

PLease make this work :-)