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

When using ViewTransition, useVisibleTask$ is not executed #80

Open
ITakayuki opened this issue Mar 29, 2024 · 7 comments
Open

When using ViewTransition, useVisibleTask$ is not executed #80

ITakayuki opened this issue Mar 29, 2024 · 7 comments

Comments

@ITakayuki
Copy link

ITakayuki commented Mar 29, 2024

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
Copy link

nakasyou commented Apr 7, 2024

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

@ITakayuki
Copy link
Author

ITakayuki commented Apr 8, 2024

@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");
  }
})

QwikDev/qwik#6084
https://discord.com/channels/842438759945601056/1224276803267330069/1224276803267330069

@nakasyou
Copy link

nakasyou commented Apr 8, 2024

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

@ITakayuki
Copy link
Author

ITakayuki commented Apr 11, 2024

@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;
  });
};

@ITakayuki ITakayuki reopened this Apr 11, 2024
@thejackshelton
Copy link
Member

thejackshelton commented Apr 13, 2024

@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
Copy link
Author

ITakayuki commented Apr 17, 2024

@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
Copy link

jkhaui commented Jun 20, 2024


<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!

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

No branches or pull requests

4 participants