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

TypeError Astro[this.getAttribute(...)] in Chrome #4286

Closed
1 task
sandervspl opened this issue Aug 12, 2022 · 16 comments · Fixed by #4375
Closed
1 task

TypeError Astro[this.getAttribute(...)] in Chrome #4286

sandervspl opened this issue Aug 12, 2022 · 16 comments · Fixed by #4375
Assignees
Labels
- P5: urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority)

Comments

@sandervspl
Copy link

sandervspl commented Aug 12, 2022

What version of astro are you using?

v1.0.3

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

pnpm

What operating system are you using?

Mac

Describe the Bug

It's not something I am able to reproduce, sadly. It only happens on Chrome (both Mac and Windows) and only when deployed (to Cloudflare in this case). I have turned off minification on Cloudflare but that didn't fix it.

The error:

Uncaught (in promise) TypeError: Astro[this.getAttribute(...)] is not a function
    at a.childrenConnectedCallback ((index):455:1910)
childrenConnectedCallback @ (index):455
await in childrenConnectedCallback (async)
connectedCallback @ (index):455
(anonymous) @ (index):455

Relevant code from source

async childrenConnectedCallback() {
  window.addEventListener("astro:hydrate", this.hydrate),
  await import(this.getAttribute("before-hydration-url"));
  const r = JSON.parse(this.getAttribute("opts"));
  Astro[this.getAttribute("client")](async()=>{
      const e = this.getAttribute("renderer-url")
        , [i,{default: o}] = await Promise.all([import(this.getAttribute("component-url")), e ? import(e) : ()=>()=>{}
      ]);
      return this.Component = i[this.getAttribute("component-export") || "default"],
      this.hydrator = o,
      this.hydrate
  }
  , r, this)
}

Link to Minimal Reproducible Example

This is one of the pages where the error occurs

https://wowvalor.app/en/death-knight/blood/mm/

Participation

  • I am willing to submit a pull request for this issue.
@bluwy
Copy link
Member

bluwy commented Aug 12, 2022

I can see the error in Chrome only too. Firefox and Safari are working fine. It looks like it's trying to access Astro.idle to only load the component after idle, but Astro.idle is only initialized after it's being called, the script tag order is incorrect. I'm not sure how this happens yet though (investigating).

@sandervspl
Copy link
Author

Thanks for looking into it!

@matthewp matthewp added - P5: urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority) s1-small labels Aug 12, 2022
@matthewp matthewp self-assigned this Aug 12, 2022
@matthewp
Copy link
Contributor

@sandervspl First, very cool app. Is it possible that you are using one of your :idle components inside of a slot? This is a very important bug to fix. Trying to figure out why it is happening because it should not. Slots is one area where I think this bug could occur. Going to attempt to recreate locally.

@matthewp
Copy link
Contributor

This is the scenario I'm thinking could cause the bug:

<One />

<Something>
  <Two client:idle />
</Something>

Where <One /> has a component that includes client:idle.

@sandervspl
Copy link
Author

sandervspl commented Aug 12, 2022

Thanks! 🙂 The entire page is wrapped with a layout, so there is definitely a slot where client:idle is being used. Here is a small sample of one of those cases.

// MenuLayout.astro

<html lang={locale || 'en'} class="h-full min-h-full">
  <body class="min-h-full">
    <main class="bg-base-300 min-h-full">
      <div class="h-full layout-container flex flex-col items-center">
        <div class="h-full w-full">
          <slot name="body" />
        </div>
      </div>
    </main>
  </body>
</html>
// /pages/.../index.astro

<MenuLayout>
  <BTFTrigger client:idle>
  ...
  </BTFTrigger>
</MenuLayout>

BTFTrigger is a Preact component.

Is this what you are thinking of?

@sandervspl
Copy link
Author

sandervspl commented Aug 12, 2022

@matthewp I have sent you a request to access the repository. That might make tracking this down a bit easier

@matthewp
Copy link
Contributor

I think I can recreate it in a test, working on that now.

@matthewp
Copy link
Contributor

@sandervspl I think what is happening is that your MenuLayout component takes a slot that contains a client:idle, and the layout has its own MainMenu being used which also contains a component using client:idle. The slot is rendered first causing the hydration script tag to be created for it and not the MainMenu component which needs it.

I think this gives me enough info to recreate in a test.

@matthewp
Copy link
Contributor

Yep, have a breaking test now, thanks!

@sandervspl
Copy link
Author

sandervspl commented Aug 12, 2022

Would it make sense to remove all :idle from children of a parent component which has :idle? Or is hydration done from child up to the parent?

@matthewp
Copy link
Contributor

Astro components can't have client: directives. If you're looking for a workaround, maybe creates a client component that renders nothing and make it be the first thing you render in your page's body, something like <Empty client:idle />, that should ensure the scripts are added before any of the components render.

But, I don't think this bug will take too long to fix, I wrote this part of the code and understand why it's breaking.

@matthewp
Copy link
Contributor

PR: #4288

@sandervspl
Copy link
Author

Thanks a lot for the quick work! 💛

@matthewp
Copy link
Contributor

Fixed! Release should go out later today.

@elevatebart
Copy link
Contributor

For those wondering why this broke again, the fix had to be reverted.
Here is your answer
#4288 (comment)

Another fix is on the way.

Thank you @matthewp for the quick turnaround.

@matthewp
Copy link
Contributor

Yes, sorry for not reopening.

The fix I made turned out not be a good one. The new fix will allow for hydration directive scripts to be defined lower than the islands that use them, because it seems that we can't control that with slots. I plan on working on this fix tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P5: urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants