TwicPics / components

A Web component library that brings the power of TwicPics to your favorite web framework.
MIT License
53 stars 2 forks source link

TwicImg preview hangs sometimes #101

Closed pfloride closed 3 months ago

pfloride commented 3 months ago

We're using @twicpics/components/sveltekit/TwicImg.svelte to render some images in our Astro.js/Svelte app. It's working mostly as desired, but the preview of TwicImg sometimes hangs / is not replaced by the actual image. Is this a known problem?

https://github.com/TwicPics/components/assets/8134422/17ee347e-9344-4613-9e10-059604fee4ad

jaubourg commented 3 months ago

Hello @pfloride,

AFAIK, this is not a known issue. Could you share a test page demonstrating the issue somewhere so that we can try, reproduce and investigate?

In the mean time: what versions of Astro.js, Svelte and TwicPics components are you using? Do you see this behavior in all browsers?

pfloride commented 3 months ago

Hi @jaubourg,

Thank you for the fast response! Turns out, that just preparing a minimal example helps a lot narrowing down the problem. :)

We've split the initialization and the viewer into two distinct components; in the example they are Loader.svelte and Viewer.svelte. Both of them are rendered by an Astro page. It feels like there's some kind of race-condition when structuring the code like this. Putting the installTwicPics call directly into the viewer component makes the problem go away. The idea behind splitting the code in the first place, was that we're probably going to use TwicPics components in more than one place and we just want to initialize once.

In Chrome and Safari the bug is easy reproducible by reloading the page a couple of times. I'm not able to reproduce with Firefox.

Dependcies:

"@astrojs/svelte": "^5.4.0",
"@twicpics/components": "0.29.0",
"astro": "^4.6.3",
"svelte": "^4.2.5"

Sample page: https://2kghpg-4321.csb.app Codesandbox: https://codesandbox.io/p/devbox/cranky-dan-2kghpg

https://github.com/TwicPics/components/assets/8134422/9b5c753a-793a-4416-8b35-0b87846c47cf

jaubourg commented 3 months ago

Ah race conditions, gotta love to hate those!

No matter the framework, installTwicPics is supposed to be called as early as possible in a project and is not meant to be part of the rendering tree. Think of it as a global configuration type of thing. It may be tempting to add the TwicPics domain as a parameter to a component but since you can only have one TwicPics domain per page, it's an anti-pattern.

If you wanna call installTwicPics only when a component uses it (which makes sense), then I'd recommend putting the TwicPics domain in the app configuration (be it svelte or astro), and have a very simple module file that imports the stylesheet and calls installTwicPics using the domain from the configuration. Then any component making use of TwicPics can import this loading module and the bundler will take care of the rest. That way, TwicPics init code should always be called before any component using it is mounted.

pfloride commented 3 months ago

The motivation behind using the Loader component was more or less exactly to initialize as early as possible independent of later Viewer initializations. Doing that inside a Svelte component was the wrong way, as we've no way to know how the execution/initialization order is going to look like.

I guess the simple and correct way to do it would be directly in the head with a script tag.

Because our stack is a bit more involved (microfrontends+SSI) and because we only want to initialize if we're actually using TwicImg (which isn't always the case) we're going with this approach, which seems to work fine too and is 'on demand':

<script lang="ts">
    import "@twicpics/components/style.css"
    import TwicImg from "@twicpics/components/sveltekit/TwicImg.svelte"
    import { installTwicPics } from "@twicpics/components/svelte4"
    import { onMount } from "svelte"

    import "../window.d.ts"

    onMount(() => {
        if (twicPicsUrl && !window.__TWICPICS_INITIALIZED__) {
            window.__TWICPICS_INITIALIZED__ = true
            installTwicPics({ domain: twicPicsUrl })
        }
    })

    export let src: string | undefined
    export let alt: string | undefined
    export let twicPicsUrl: string | undefined
</script>

<TwicImg {alt} {src} ratio="none" />

Thank you again for the responsiveness, that one got solved faster than I anticipated ;)