QwikDev / astro

Qwik + Astro integration
189 stars 13 forks source link

When using ViewTransition, `useVisibleTask$` is not executed #80

Open ITakayuki opened 3 months ago

ITakayuki commented 3 months ago

When using useVisibleTask$ in a Qwik component, if <ViewTransition/> is set, useVisibleTask$ is not executed on page transitions. track, etc. will not work as well. Is there any workaround for this? ex)

import { component$, useVisibleTask$ } from "@builder.io/qwik";

export default component$(() => {

  // Not executed when transitioned by ViewTransition
  useVisibleTask$(() => {
    console.log("Working useVisibleTask$");
  });
  return /* .... */
});
@builder.io/qwik 1.5.1
@qwikdev/astro 0.5.13
astro 4.5.12

CodeSandBox

nakasyou commented 2 months ago

I met this trouble. @ITakayuki, how could you resolve it probrem?

ITakayuki commented 2 months ago

@nakasyou I checked with Discord and will fix it on the @BuilderIO/qwik side of the scope.

For now, as a tentative response, I used astro:page-load on the Hook to update Qwik's Context and useTask$ is executed, so I track the Context and created a pseudo useVisibleTask$.

// Qwik State Manage Component

useOnDocument("pageLoad", $(() => {
  // update pageLoadState Context
}))
// Astro Layout

<!--- Qwik State Manage Component --->
<WatchPageState/>
<script>
  document.addEventListener('astro:page-load', function () {
    document.dispatchEvent(new CustomEvent("pageLoad"));
  });
</script>
// Qwik Component
useTask$(({track}) => {
  track(() => pageLoadState.value);
  if (isBrowser) {
    console.log("like useVisibleTask");
  }
})

https://github.com/BuilderIO/qwik/issues/6084
https://discord.com/channels/842438759945601056/1224276803267330069/1224276803267330069

nakasyou commented 2 months ago

Hi @ITakayuki, thank you for your helping! I resolved it problem! I hope that solve it fundamentally.

ITakayuki commented 2 months ago

@QwikDev/qwik seems difficult, so how about the following solution?

declare global {
  interface Window {
    __astro_spa__: boolean;
  }

  interface Document {
    __q_context__: any;
  }
}

const emitEvent = <T extends CustomEvent = any>(
  eventName: string,
  detail?: T["detail"],
) => {
  document.dispatchEvent(new CustomEvent<T>(eventName, { detail }));
};
const resolveContainer = (containerEl: Element) => {
  if ((containerEl as any)._qwikjson_ === undefined) {
    const parentJSON =
      containerEl === document.documentElement ? document.body : containerEl;
    let script = parentJSON.lastElementChild;
    while (script) {
      if (
        script.tagName === "SCRIPT" &&
        script.getAttribute("type") === "qwik/json"
      ) {
        (containerEl as any)._qwikjson_ = JSON.parse(
          script.textContent!.replace(/\\x3C(\/?script)/gi, "<$1"),
        );
        break;
      }
      script = script.previousElementSibling;
    }
  }
};
const dispatch = async (
  element: Element,
  onPrefix: string,
  ev: Event,
  eventName = ev.type,
) => {
  const attrName = "on" + onPrefix + ":" + eventName;
  if (element.hasAttribute("preventdefault:" + eventName)) {
    ev.preventDefault();
  }
  const ctx = (element as any)["_qc_"] as any | undefined;
  const relevantListeners =
    ctx && ctx.li.filter((li: any) => li[0] === attrName);
  if (relevantListeners && relevantListeners.length > 0) {
    for (const listener of relevantListeners) {
      // listener[1] holds the QRL
      await listener[1].getFn([element, ev], () => element.isConnected)(
        ev,
        element,
      );
    }
    return;
  }
  const attrValue = element.getAttribute(attrName);
  if (attrValue) {
    const container = element.closest("[q\\:container]")!;
    const base = new URL(container.getAttribute("q:base")!, document.baseURI);
    for (const qrl of attrValue.split("\n")) {
      const url = new URL(qrl, base);
      const symbolName = url.hash.replace(/^#?([^?[|]*).*$/, "$1") || "default";
      const reqTime = performance.now();
      let handler: any;
      const isSync = qrl.startsWith("#");
      if (isSync) {
        handler = ((container as any).qFuncs || [])[
          Number.parseInt(symbolName)
        ];
      } else {
        const module = import(/* @vite-ignore */ url.href.split("#")[0]);
        resolveContainer(container);
        handler = (await module)[symbolName];
      }
      const previousCtx = document.__q_context__;
      if (element.isConnected) {
        try {
          document.__q_context__ = [element, ev, url];
          isSync ||
            emitEvent("qsymbol", {
              symbol: symbolName,
              element: element,
              reqTime,
            });
          await handler(ev, element);
        } finally {
          document.__q_context__ = previousCtx;
        }
      }
    }
  }
};

export const astroQwikSPA = () => {
  document.addEventListener("astro:after-swap", () => {
    window.__astro_spa__ = true;
  });
  document.addEventListener("astro:page-load", () => {
    const results = document.querySelectorAll("[on\\:" + "qvisible" + "]");
    emitEvent("qinit");
    const riC = window.setTimeout;
    riC.bind(window)(() => emitEvent("qidle"));
    if (!window.__astro_spa__) return;
    if (results.length !== 0) {
      const observer = new IntersectionObserver((entries) => {
        for (const entry of entries) {
          if (entry.isIntersecting) {
            observer.unobserve(entry.target);
            dispatch(
              entry.target,
              "",
              new CustomEvent("qvisible", { detail: entry }),
            );
          }
        }
      });
      results.forEach((el) => observer.observe(el));
    }
    window.__astro_spa__ = false;
  });
};
thejackshelton commented 2 months ago

@QwikDev/qwik seems difficult, so how about the following solution?

declare global {
  interface Window {
    __astro_spa__: boolean;
  }

  interface Document {
    __q_context__: any;
  }
}

const emitEvent = <T extends CustomEvent = any>(
  eventName: string,
  detail?: T["detail"],
) => {
  document.dispatchEvent(new CustomEvent<T>(eventName, { detail }));
};
const resolveContainer = (containerEl: Element) => {
  if ((containerEl as any)._qwikjson_ === undefined) {
    const parentJSON =
      containerEl === document.documentElement ? document.body : containerEl;
    let script = parentJSON.lastElementChild;
    while (script) {
      if (
        script.tagName === "SCRIPT" &&
        script.getAttribute("type") === "qwik/json"
      ) {
        (containerEl as any)._qwikjson_ = JSON.parse(
          script.textContent!.replace(/\\x3C(\/?script)/gi, "<$1"),
        );
        break;
      }
      script = script.previousElementSibling;
    }
  }
};
const dispatch = async (
  element: Element,
  onPrefix: string,
  ev: Event,
  eventName = ev.type,
) => {
  const attrName = "on" + onPrefix + ":" + eventName;
  if (element.hasAttribute("preventdefault:" + eventName)) {
    ev.preventDefault();
  }
  const ctx = (element as any)["_qc_"] as any | undefined;
  const relevantListeners =
    ctx && ctx.li.filter((li: any) => li[0] === attrName);
  if (relevantListeners && relevantListeners.length > 0) {
    for (const listener of relevantListeners) {
      // listener[1] holds the QRL
      await listener[1].getFn([element, ev], () => element.isConnected)(
        ev,
        element,
      );
    }
    return;
  }
  const attrValue = element.getAttribute(attrName);
  if (attrValue) {
    const container = element.closest("[q\\:container]")!;
    const base = new URL(container.getAttribute("q:base")!, document.baseURI);
    for (const qrl of attrValue.split("\n")) {
      const url = new URL(qrl, base);
      const symbolName = url.hash.replace(/^#?([^?[|]*).*$/, "$1") || "default";
      const reqTime = performance.now();
      let handler: any;
      const isSync = qrl.startsWith("#");
      if (isSync) {
        handler = ((container as any).qFuncs || [])[
          Number.parseInt(symbolName)
        ];
      } else {
        const module = import(/* @vite-ignore */ url.href.split("#")[0]);
        resolveContainer(container);
        handler = (await module)[symbolName];
      }
      const previousCtx = document.__q_context__;
      if (element.isConnected) {
        try {
          document.__q_context__ = [element, ev, url];
          isSync ||
            emitEvent("qsymbol", {
              symbol: symbolName,
              element: element,
              reqTime,
            });
          await handler(ev, element);
        } finally {
          document.__q_context__ = previousCtx;
        }
      }
    }
  }
};

export const astroQwikSPA = () => {
  document.addEventListener("astro:after-swap", () => {
    window.__astro_spa__ = true;
  });
  document.addEventListener("astro:page-load", () => {
    const results = document.querySelectorAll("[on\\:" + "qvisible" + "]");
    emitEvent("qinit");
    const riC = window.setTimeout;
    riC.bind(window)(() => emitEvent("qidle"));
    if (!window.__astro_spa__) return;
    if (results.length !== 0) {
      const observer = new IntersectionObserver((entries) => {
        for (const entry of entries) {
          if (entry.isIntersecting) {
            observer.unobserve(entry.target);
            dispatch(
              entry.target,
              "",
              new CustomEvent("qvisible", { detail: entry }),
            );
          }
        }
      });
      results.forEach((el) => observer.observe(el));
    }
    window.__astro_spa__ = false;
  });
};

Hey @ITakayuki! First off, thanks for taking an initiative to fix this issue! Yeah this is a tough one.

Feel free to make a pull request with these changes.

Before you do so, can you explain why each piece is needed? And also why you think that belongs here instead of core?

If this was to be fixed in core (especially if view transitions have the same issue there), wouldn't this be duplicate code?

This is a pretty sizable amount of code for this one fix in the integration. I'm not saying it is not needed, but it is definitely intimidating, and raises potential issues down the road.

What I've tried to do so far, is seeing if data-astro-rerun on the qwik scripts would create a new intersection observer. Which it seems is not the case.

Maybe there's a way to hook into the qwik internals here and "reload" the intersection observers.

ITakayuki commented 2 months ago

@thejackshelton Sorry for the late reply.

And also why you think that belongs here instead of core?

This is the logic implemented on the Qwik City side if Qwik City + Qwik. For Astro + Qwik, I believe Astro equivalent to Qwik City, so the logic in VT is an issue here.

If this was to be fixed in core (especially if view transitions have the same issue there), wouldn't this be duplicate code? This is a pretty sizable amount of code for this one fix in the integration. I'm not saying it is not needed, but it is definitely intimidating, and raises potential issues down the road.

I don't think that the logic tied to ViewTransition is implemented on the Qwik Core side. Because I think Qwik or the underlying functionality, and the role of the upper framework is to wrap and extend them.

What I've tried to do so far, is seeing if data-astro-rerun on the qwik scripts would create a new intersection observer. Which it seems is not the case. Maybe there's a way to hook into the qwik internals here and "reload" the intersection observers.

That is certainly true. However, in the code I wrote above, there were still various problems, such as cleanup not working. I think this is due to components not being unmounted during VT transitions. So we need to consider removing the components that are displayed. For example, how about adding a useVTVisibleTask$/useVTTask$ on the qwikdev/astro side that is activated by the astro hook? What about creating a wrapped use function for astro's VT that is not too influenced by the qwik core?

Thanks.

jkhaui commented 1 week ago

<script>
  document.addEventListener('astro:page-load', function () {
    document.dispatchEvent(new CustomEvent("pageLoad"));
  });
</script>

Hey @nakasyou or @ITakayuki , are you able to please provide more info on how to implement this with Qwik's state management?

I'm new to Qwik (been a React dev for many years). Been trying to get a "qwikified" React island to hydrate on a VT route change and this looks like the closest I'll get to a solution.

Following your answer, my React brain interprets it as first creating a WatchPageState component like so:

import {$, component$, useContextProvider, useOnDocument, useSignal} from "@builder.io/qwik";
import {PathnameCtx} from "./use-pathname.ts";
import {isBrowser} from "@builder.io/qwik/build";

export const WatchPageState = component$(() => {
    const pageSignal = useSignal('')

    useContextProvider(PathnameCtx, pageSignal)

    useOnDocument("pageLoad", $(() => {
        console.log(window.location.pathname, 11)

        if (isBrowser) {
            pageSignal.value = window.location.pathname;
        }
    }))

    return null;
})

Is this correct so far? After placing this in the head of my layout (exactly as you've done with the custom pageLoad event dispatched on Astro's own page load event), I can see the console log printing fine in the browser.

My question is now: how do I then "track" this value from a Qwik component inside the document body? From what I understand I can't use Qwik context because WatchPageState is a standalone component (and therefore can't wrap children component, so its context value can never be subscribed to).

Any suggestions on what I do from here or correcting my implementation is much appreciated 🙏 thank you!