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

Support SSR (Reopen #9317) #9385

Merged
merged 9 commits into from
Jul 6, 2024
Merged

Support SSR (Reopen #9317) #9385

merged 9 commits into from
Jul 6, 2024

Conversation

Falke-Design
Copy link
Member

I accidentally closed the other PR #9317 ... Look for more details into it

I added the changes from @jonkoops and made it work with Deno.

@Gugustinette can you please test this version?

@Falke-Design Falke-Design mentioned this pull request Jul 2, 2024
@Gugustinette
Copy link

I'll look into that asap !

@Gugustinette
Copy link

Gugustinette commented Jul 3, 2024

Just tested the branch in Nuxt Leaflet playground, and it works like a charm !

Here is the example code (some Vue code) :

<template>
  <div style="height:100vh; width:100vw">
    <h1>Basic Map Exemple</h1>
    <LMap
      ref="map"
      :zoom="6"
      :max-zoom="18"
      :center="[47.21322, -1.559482]"
      :use-global-leaflet="false"
      @ready="onMapReady"
    >
      <LTileLayer
        url="https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"
        attribution="&amp;copy; <a href=&quot;https://www.openstreetmap.org/&quot;>OpenStreetMap</a> contributors"
        layer-type="base"
        name="OpenStreetMap"
      />
    </LMap>
  </div>
</template>

<script setup lang="ts">
import L from 'leaflet'
import { ref } from 'vue'

const map = ref(null)

// When the map is ready
const onMapReady = () => {
  console.log('Map is ready')
  console.log(L)
  console.log(map.value.maxZoom)
}
</script>

Before, an error was thrown as SSR environment will typically load (and optimize) imports on the server-side :

Capture d’écran 2024-07-03 à 13 36 32

After, the same code now works correctly :

Capture d’écran 2024-07-03 à 13 33 41

Leaflet instance can now be loaded in a node server context.
Huge step forward for SSR environments 👍

@Falke-Design
Copy link
Member Author

Falke-Design commented Jul 3, 2024

@mourner @jonkoops I tried to implement a test in the workflow. This was my first change in a workflow, can you please check it?

@Falke-Design Falke-Design requested a review from mourner July 3, 2024 20:20
.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
Copy link
Collaborator

@jonkoops jonkoops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of small optional comments, but this is good to merge AFAIK

.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
spec/ssr/ssr_node.mjs Show resolved Hide resolved
src/core/Browser.js Show resolved Hide resolved
@Falke-Design Falke-Design requested a review from jonkoops July 5, 2024 18:22
@jonkoops jonkoops merged commit 35d79e0 into Leaflet:main Jul 6, 2024
18 checks passed
@Teebo
Copy link

Teebo commented Aug 23, 2024

Hi, when should we expect this to be released?
v1.9.4 is still the latest in npm and it does not include this fix.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants