Support SSR

[matthova]

This PR is an alternative to #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:

• Feature/ssr #7085
• Wrap references to window to prevent them from erroring in node.js environment #6332
• Error on server side prerendering using leaflet (Window undefined) #5402

[mourner]

The changes looks good to me! Fairly minimal for enabling a rare but increasing common use case. The only thing I'd suggest is seeing if we can add some kind of a test so that we don't accidentally regress on this in future PRs (which seems easy).

[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.

[jonkoops]

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.

Not sure what we want to do with those, handling them on a server is not really possible, and the guard code would be hundreds of lines of code.

[jonkoops]

This should take into account that window.devicePixelRatio and window.screen might be undefined in some run-times that do support a global window, such as Deno. Since window.devicePixelRatio is supported in all browsers we want to support for v2, I propose we drop the fallback here.

const retina = typeof window === 'undefined' || typeof window.devicePixelRatio === 'undefined' ? false : window.devicePixelRatio > 1;

[jonkoops]

This is a personal preference, but I prefer to guard the clause rather than to fall back to default values.

Suggested change

	const mac = typeof navigator === 'undefined' ? false : (navigator?.platform ?? '').startsWith('Mac');
	const mac = typeof navigator === 'undefined' || typeof navigator.platform === 'undefined' ? false : navigator.platform.startsWith('Mac');

[jonkoops]

Same here, I'd prefer a guard clause than a fallback.

Suggested change

	const linux = typeof navigator === 'undefined' ? false : (navigator?.platform ?? '').startsWith('Linux');
	const linux = typeof navigator === 'undefined' || typeof navigator.plaform === 'undefined' : navigator.platfom.startsWith('Linux');

[jonkoops]

Although supported in Deno, I am not sure navigator.userAgent might be defined in other run-times, so we might as well guard this code.

Suggested change

	if (typeof navigator === 'undefined') {
	if (typeof navigator === 'undefined' || typeof navigator.userAgent === 'undefined') {

[jonkoops]

The guard here should also be more extensive, as window might be defined, but it's members not.

Suggested change

	const requestFn = typeof window === 'undefined' ? falseFn : window.requestAnimationFrame;
	const cancelFn = typeof window === 'undefined' ? falseFn : window.cancelAnimationFrame;
	const requestFn = typeof window === 'undefined' || typeof window.requestAnimationFrame === 'undefined' ? falseFn : window.requestAnimationFrame;
	const cancelFn = typeof window === 'undefined' || typeof window.cancelAnimationFrame === 'undefined' ? falseFn : window.cancelAnimationFrame;

[mourner]

@jonkoops I don't think we should be that thorough here — IMO we should do the minimum possible change that makes the bundle itself execute without errors, disregarding how it will be used in such environment. Putting too much guards just bloats the code for cases no one will run into.

[jonkoops]

Fair, just want to prevent that in this case things become polymorphic. In this case function and undefined.

[jonkoops]

I'd rather fall back to undefined here than give the impression there is a valid object with a fallback, and then use a guard clause wherever documentStyle is accessed.

[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 :-)