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

🐛 No good error message when adding a client directive to an Astro component #4090

Closed
1 task
sandervspl opened this issue Jul 29, 2022 · 4 comments · Fixed by #4201 or #5501
Closed
1 task

🐛 No good error message when adding a client directive to an Astro component #4090

sandervspl opened this issue Jul 29, 2022 · 4 comments · Fixed by #4201 or #5501
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: errors Related to error handling / messages (scope)

Comments

@sandervspl
Copy link

sandervspl commented Jul 29, 2022

What version of astro are you using?

1.0.0-rc.2

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

No

What package manager are you using?

pnpm

What operating system are you using?

Mac

Describe the Bug

When you have a Preact component and a regular Astro component in the same project, and both have client:idle (or any other version of this), it will run into a syntax error on Safari. The location of the components does not matter. They can be in different pages, or on the same page. This bug seems to only happen after a production build, and only on Safari. Both desktop and mobile versions. It blocks all JavaScript from running.

Adding a <script> tag, a frontmatter, etc, does not resolve the issue. The reproduction is the most simple example I could create. Adding more code will still result in the error.

I have not tried using other integrations. I have only tested it with Preact.

Reproduction steps

  1. Download and install the linked reproduction example, package manager doesn't matter.
  2. Open Safari
  3. Go to http://localhost:3000
  4. Open terminal

There should be an error in the terminal

Unhandled Promise Rejection: SyntaxError: Invalid regular expression: invalid group specifier name

It seems to point to this part of the code

<script>(self.Astro=self.Astro||{}).idle=a=>{const e=async()=>{await(await a())()}; ...

It specifically points to this part await a()

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-bzqxfw?file=src%2Fcomponents%2FPreactComponent.tsx,src%2Fpages%2Findex.astro&on=stackblitz

Participation

  • I am willing to submit a pull request for this issue.
@matthewp matthewp self-assigned this Jul 29, 2022
@matthewp matthewp added the - P5: urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority) label Jul 29, 2022
@Remcoman
Copy link

Remcoman commented Jul 29, 2022

I think this Regex is the issue:

k.trim().replace(/(?:(?<!^)\b\w|\s+|[^\w]+)/g, (match, index) => {

Negative-lookbehind regexes are not supported in Safari.

@matthewp What is the reason for the lookbehind? I don't notice any difference when I remove it.

@matthewp
Copy link
Contributor

That code doesn't run in the browser.

@matthewp
Copy link
Contributor

Oh. You have <AstroComponent client:idle />. Astro components cannot run in the client. I can't believe we don't have a proper error message for this already.

@matthewp matthewp added - P3: minor bug An edge case that only affects very specific usage (priority) and removed - P5: urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority) labels Jul 29, 2022
@matthewp matthewp changed the title 🐛 BUG: "invalid group specifier name" when using Preact. Safari only. 🐛 No good error message when adding a client directive to an Astro component Jul 29, 2022
@hippotastic
Copy link
Contributor

hippotastic commented Aug 9, 2022

I think having the warning message is great, and it seems to work in dev. I think we can still improve on that in two aspects though:

  1. The message does not appear during build, and therefore errors like this can go completely unnoticed, especially during CI. I'd love to see this during build as well.
  2. The message does not include the source location this error is caused by. Current output is this:
    You are attempting to render <Search client:idle />, but Search is an Astro component. Astro components do not render in the client and should not have a hydration directive. Please use a framework component for client rendering.
    This is great already, but adding the file name, and if possible also the location inside the file, would be the cherry on top. :)

I'm reopening this because it's a very recent fix and still on topic IMHO. If you think the potential improvements should be handled in a new bug, please let me know and I'll create a new one! :)

@hippotastic hippotastic reopened this Aug 9, 2022
@matthewp matthewp removed their assignment Sep 6, 2022
@Princesseuh Princesseuh added the feat: errors Related to error handling / messages (scope) label Nov 9, 2022
@Princesseuh Princesseuh self-assigned this Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: errors Related to error handling / messages (scope)
Projects
None yet
5 participants