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

astro:page-load broken and inconsistent, breaks scripts #12858

Open
1 task
cabaucom376 opened this issue Dec 30, 2024 · 13 comments
Open
1 task

astro:page-load broken and inconsistent, breaks scripts #12858

cabaucom376 opened this issue Dec 30, 2024 · 13 comments
Labels
- P2: has workaround Bug, but has workaround (priority)

Comments

@cabaucom376
Copy link

cabaucom376 commented Dec 30, 2024

Astro Info

Astro                    v5.1.1
Node                     v23.5.0
System                   macOS (arm64)
Package Manager          npm
Output                   static
Adapter                  @astrojs/node
Integrations             astro-meta-tags

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Initially, I could only get this to reproduce on Safari as it happens almost every time (especially iOS), but inconsistently, Chrome also has the bug. Sometimes when navigating to a page for the first time, astro:page-load is not triggered. After refreshing the page or navigating away and back, it triggers twice.

Utilizing the browser’s navigation controls instead of links seems to cause this more consistently. Whenever on a page with an astro:page-load script and you utilize browser controls to go back to a previous page, it will fire off the same script again, causing unintended side effects, especially if using event listeners.

What's the expected result?

astro:page-load only triggers once and is consistent. Utilizing browser navigation controls should not interfere as well.

Link to Minimal Reproducible Example

example

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Dec 30, 2024
@martrapp martrapp added the needs repro Issue needs a reproduction label Dec 30, 2024
Copy link
Contributor

Hello @cabaucom376. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with needs repro will be closed if they have no activity within 3 days.

@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Dec 30, 2024
@cabaucom376
Copy link
Author

@martrapp Added an example. I don't seem to see any logic working on stackblitz but the code works locally. To reproduce:

  1. navigate to test page using link
  2. navigate back to home, triggering a re-execution
  3. navigate back to test page

@ematipico ematipico added needs triage Issue needs to be triaged and removed needs repro Issue needs a reproduction labels Jan 2, 2025
@martrapp
Copy link
Member

I am sorry, for the delay @cabaucom376, i somehow missed your update 😞
Could you please add the ClientRouter to your base.astro?

@cabaucom376
Copy link
Author

cabaucom376 commented Jan 11, 2025

@martrapp Ah yeah, I missed that in the example. Thats not the source of my issues in production though. This following code is my exact issue. If you want to mess with it go to https://prizm.sh, tap on "Explore Features" and then make sure your browser is small enough to trigger the smaller breakpoint showing the mobile menu button. That button will not work on the first load until you reload the page. You can repeat this over and over by going back to the "/" homepage and hard refreshing. You can observe this same behavior on Safari iOS as well if you also want to see it there.

base.astro:

---
import { ClientRouter } from "astro:transitions";
import { SITE, META, CONTACT, SOCIAL } from "@constants";
import "@styles/global.css";
import "@styles/tailwind.css";

interface Props {
  title?: string;
  description?: string;
  icon?: string;
  image?: string;
  url?: string;
}

const {
  title = SITE.TITLE,
  description = SITE.DESCRIPTION,
  icon = SITE.ICON,
  image = SITE.DEFAULT_OG_IMAGE,
  url = SITE.URL,
} = Astro.props;
---

<!doctype html>
<html lang="en" data-theme="dark" class="scroll-smooth">
  <head>
    <meta charset="UTF-8" />
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
    <meta
      name="viewport"
      content="width=device-width, initial-scale=1.0, maximum-scale=5.0"
    />

    <!-- Search Engine and Browser Meta Tags -->
    <meta name="description" content={description} />
    <meta name="abstract" content={SITE.ABSTRACT} />

    <!-- Open Graph Meta Tags -->
    <meta property="og:image" content={image} />
    <meta property="og:image:alt" content={description} />
    <meta property="og:site_name" content={SITE.TITLE} />
    <meta property="og:title" content={title} />
    <meta property="og:description" content={description} />
    <meta property="og:type" content="website" />
    <meta property="og:url" content={url} />
    <meta property="og:locale" content={SITE.LOCALE} />
    <meta property="og:email" content={CONTACT.EMAIL} />

    <!-- Twitter Meta Tags -->
    <meta name="twitter:card" content="summary_large_image" />
    <meta name="twitter:title" content={title} />
    <meta name="twitter:description" content={description} />
    <meta name="twitter:image" content={image} />
    <meta name="twitter:url" content={url} />
    <meta name="twitter:site" content={SOCIAL.TWITTER} />

    <!-- Additional Meta Tags -->
    <meta name="generator" content={Astro.generator || "Astro"} />
    <meta name="author" content={META.AUTHOR} />
    <meta name="theme-color" content={META.DARK_THEME_COLOR} />
    <link rel="icon" type="image/svg+xml" href={icon} />
    <link rel="canonical" href={url} />

    <!-- Title -->
    <title>{title}</title>

    <ClientRouter />
  </head>

  <body
    class="bg-background text-on-background selection:bg-secondary/30 overscroll-none"
  >
    <slot />
  </body>
</html>

trouble page:

---
import { getCollection } from "astro:content";
import MainLayout from "@layouts/MainLayout.astro";
import ActionButton from "@components/ActionButton.astro";
import FeatureStatusChip from "@components/FeatureStatusChip.astro";

const categories = (await getCollection("categories")).sort(
  (a, b) => a.data.sequence - b.data.sequence
);
const features = (await getCollection("features")).sort(
  (a, b) => a.data.sequence - b.data.sequence
);
---

<MainLayout title="Prizm - Features">
  <div id="top"></div>
  <div
    class="max-w-container mx-auto box-content w-[90%] space-y-8 px-4 py-28 sm:w-[80%] sm:px-12 md:px-16 md:py-48 lg:space-y-16 lg:px-24"
  >
    <!-- Introduction -->
    ...

    <!-- Features -->
    <div id="features" class="flex lg:gap-12">
      <!-- Desktop Sidebar -->
      <aside id="desktop-sidebar" class="w-[20%] max-lg:hidden">
        <div
          id="overview"
          class="sticky top-0 flex flex-col gap-6 pt-10 sm:pt-16 md:pt-24"
        >
          <ul>
            <li class="mb-4">
              <ActionButton href="#top" size="sm">Back to top</ActionButton>
            </li>
            {
              categories.map(({ data: { title } }) => {
                return (
                  <li>
                    <a
                      href={`#${title.toLowerCase()}`}
                      class="group hover:bg-surface flex items-center gap-4 rounded-lg px-3.5 py-2.5 tracking-tight transition-colors duration-300"
                    >
                      {title}
                    </a>
                  </li>
                );
              })
            }
          </ul>
        </div>
      </aside>

      <!-- Features -->
      <ul id="features" class="w-full lg:w-[80%]">
        {
          categories.map(({ id, data: categoryData }) => {
            const categorizedFeatures = features.filter(
              (feature) => feature.data.category?.id === id
            );

            return (
              <>
                <hr class="border-surface-variant border-2 md:my-12" />
                <li id={id} class="scroll-m-24 pb-12 lg:pb-16">
                  <h2 class="mt-10 text-4xl lg:text-5xl">
                    {categoryData.title}
                  </h2>
                  <div class="mt-10 gap-6 space-y-6 xl:grid xl:grid-cols-2">
                    {categorizedFeatures.map(({ id, data: featureData }) => {
                      return (
                        <ul class="space-y-4">
                          <li class="flex flex-row gap-4 md:items-center">
                            <h4 class="text-xl lg:text-3xl">
                              {featureData.title}
                            </h4>
                            <FeatureStatusChip
                              featureId={id}
                              status={featureData.status}
                            />
                          </li>
                          <p class="text-on-background/60 text-base lg:text-lg">
                            {featureData.description}
                          </p>
                        </ul>
                      );
                    })}
                  </div>
                </li>
              </>
            );
          })
        }
      </ul>

      <!-- Mobile Overview Button -->
      <button
        id="overview-button"
        class="bg-surface/60 text-on-background hover:bg-surface fixed right-6 bottom-6 z-50 flex h-14 w-14 items-center justify-center rounded-2xl shadow-md backdrop-blur-md transition lg:hidden"
      >
        <svg
          xmlns="http://www.w3.org/2000/svg"
          fill="none"
          viewBox="0 0 24 24"
          stroke="currentColor"
          class="h-6 w-6"
          aria-hidden="true"
        >
          <path
            stroke-linecap="round"
            stroke-linejoin="round"
            stroke-width="2"
            d="M4 6h16M4 12h16m-7 6h7"></path>
        </svg>
        <span class="sr-only">Table of Contents</span>
      </button>

      <!-- Mobile Overview Pane -->
      <div
        id="overview-pane"
        class="bg-background/90 fixed inset-0 z-40 hidden p-6 backdrop-blur-md lg:hidden"
      >
        <nav class="mx-auto max-w-lg space-y-4">
          <ul class="space-y-2">
            <li data-overview-link>
              <ActionButton href="#top" size="md">Back to top</ActionButton>
            </li>
            {
              categories.map(({ data: { title } }) => (
                <li data-overview-link>
                  <a
                    href={`#${title.toLowerCase()}`}
                    class="bg-surface/40 hover:bg-surface/80 block rounded-lg px-4 py-3 shadow-md transition"
                  >
                    {title}
                  </a>
                </li>
              ))
            }
          </ul>
        </nav>
      </div>
    </div>
  </div>
  <script>
    document.addEventListener("astro:page-load", () => {
      const body = document.body;
      const overviewButton = document.querySelector("#overview-button");
      const overviewPane = document.querySelector("#overview-pane");

      if (overviewButton) {
        overviewButton.addEventListener("click", () => {
          overviewPane?.classList.toggle("hidden");
          body.style.overflow = overviewPane?.classList.contains("hidden")
            ? ""
            : "hidden";
        });

        overviewPane
          ?.querySelectorAll("[data-overview-link]")
          .forEach((link) => {
            link.addEventListener("click", () => {
              overviewPane.classList.add("hidden");
              body.style.overflow = "";
            });
          });
      }
    });
  </script>
</MainLayout>

@martrapp
Copy link
Member

Yes. I know that this was not your problem. But could you please fix the minimal reproduction though that other people looking at this issue can start helping right away and are not forced to first puzzle the code together? Just add the router please. I cant do that in your stackblitz. And please stay minimal. We do not need more code from your site.

@cabaucom376
Copy link
Author

Okay its updated

@martrapp
Copy link
Member

Thank you! I'll be right back ;-)

@martrapp martrapp removed the needs triage Issue needs to be triaged label Jan 11, 2025
@martrapp
Copy link
Member

Hi @cabaucom376,

your example works fine if executed locally with npm run dev. It shows two different issues when build (npm run build) and served with npm run preview

a) the astro:page-load listener is not called on the first visit of the test page.
b) when going back and forth from test to home, listeners pile up.

b) is a duplicate of #12804

I think a) is a consequence of this change in Astro 5 but i need a bit more time to analyze.

As a quick fix, in your astro.config.mjs, please set vite.buildassetsInlineLimit to 0 like so:

import { defineConfig } from 'astro/config';
export default defineConfig({
  vite: {
    build: {
      assetsInlineLimit: 0
    },
  },
});

This should fix both issues for you at the cost of disabling asset inlining.

@martrapp
Copy link
Member

Analysis for astro:page-load not working:

The event is astro:page-loed dispatched before the listener is added.
The client router waits for all external scripts to execute before it dispatches astro:page-load.
It does not wait for inline scripts, assuming that they execute immediately.
This is not true for inline module scripts.

Best fix for user code currently is to set the assetsInlineLimit to 0 as described above.
This avoids automatic script inlining and in the example of this issue it avoids the problem of the listener being added too late.

@cabaucom376, please let me know whether setting the limit worked for you.

@cabaucom376
Copy link
Author

@martrapp thank you so much! I was struggling to even grasp what was going on here and I'm glad to see more details between what you've found and discussed in that other issue. I've already pushed the changes and verified that it indeed did fix my problem.

@martrapp
Copy link
Member

Glad I could help!
Apologies again for the delay!
Feel free to ping me on Discord if I seem unresponsive. 😊

@martrapp martrapp added the - P2: has workaround Bug, but has workaround (priority) label Jan 11, 2025
@bonno123
Copy link

bonno123 commented Jan 13, 2025

I observed here that in my safari (using v17.6) when opting out ClientRouter with built-in animation option=none <ClientRouter fallback="none" />

It does not dispatch any event for astro:page-load while navigation changes: force me back to use either of the rest two fallback options animate or swap to execute the code inside that block.

@martrapp
Copy link
Member

Hi @bonno123, yes you need to select at least "swap" for the life cycle events. With "none"

you will get full page navigation in non-supporting browsers.

see https://docs.astro.build/en/guides/view-transitions/#fallback-control, which means the default browser behavior without astro: events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: has workaround Bug, but has workaround (priority)
Projects
None yet
Development

No branches or pull requests

4 participants