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

Nextjs builds fail with Sentry and headlessui #611

Closed
brandinchiu opened this issue Sep 23, 2024 · 8 comments
Closed

Nextjs builds fail with Sentry and headlessui #611

brandinchiu opened this issue Sep 23, 2024 · 8 comments

Comments

@brandinchiu
Copy link

Environment

What version are you running? Etc.
"@headlessui/react": "^2.1.5",
"@heroicons/react": "^2.1.5",
"@sentry/nextjs": "^8.28.0",
"next": "14.2.8",
"react": "^18",
"react-dom": "^18"

I've created a point-in-time backup of my current working project here: https://github.com/brandinchiu/sentry-build-error

Disabling reactComponentAnnotation will allow the build to complete successfully.

Steps to Reproduce

  1. followed this tutorial to get Sentry installed on a blank Nextjs project: https://docs.sentry.io/platforms/javascript/guides/nextjs/
  2. create a page in Nextjs that makes use of a component from headlessui/react
  3. attempt to build the application with reactComponentAnnotation enabled.

Expected Result

Build should complete.

Actual Result

Build fails. Error is throw indicating that Nextjs prerendering failed when trying to inject Sentry annotations into components:

Error occurred prerendering page "/". Read more: https://nextjs.org/docs/messages/prerender-error

Error: Passing props on "Fragment"!

The current component <Menu /> is rendering a "Fragment".
However we need to passthrough the following props:
  - data-sentry-element
  - data-sentry-component
  - data-sentry-source-file
  - data-headlessui-state

You can apply a few solutions:
  - Add an `as="..."` prop, to ensure that we render an actual element instead of a "Fragment".
  - Render a single element as the child so that we can forward the props onto that element.
...
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Sep 23, 2024
@chargome
Copy link
Member

Hi @brandinchiu, have you tried any of the solutions of not using a fragment?

@brandinchiu
Copy link
Author

It's an external UI kit, so I have limited control in this case.

Regardless, fragments are a fairly common react construct. Causing all builds to fail if one exists anywhere in the component tree is a bit extreme, imo.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Sep 24, 2024
@chargome
Copy link
Member

@brandinchiu
Yes, but the error is not originating from Sentry but from headless ui. You could just render the headless menu as a div for example: <Headless.Menu as="div" {...props} />;

This should fix your error 👍

@brandinchiu
Copy link
Author

Respectfully, I disagree with the premise that this is a headlessui issue. I don't even think it's a Sentry issue.

But the Venn diagram here creates a legitimate problem, and I just can't see the solution being "never use a naked fragment as long as Sentry is installed".

Or, this needs to be clearly documented as a drawback that anyone who uses react can't use fragments without an as clause with Sentry (if this IS already documented somewhere, please share a link).

I am not working directly with headlessui. I am working with another UI kit that builds on top of headlessui.

Your suggestion above suggests that using a fragment without an as prop is wrong, and the best solution here is to just never do it.

As I mentioned in my first reply, this feels exceptionally harsh.

@chargome
Copy link
Member

Well the thrown error from your reproduction repo is coming from headless ui and that is what I pointed out in my previous comment.
I'm not suggesting that you should not be using any fragments, but we cannot change the fact that React fragments are not accepting props. So this is rather an implementation detail on how to pass down props to child components and this implementation detail in this case lies in headless ui – for which use case their as prop could be used.

@brandinchiu
Copy link
Author

Fair enough. My solution for now will be to simply disable reactComponentAnnotation, as getting into guts of the UI kit I'm using would kind of defeat the purpose of offloading this work.

I'll spend some time with it at a later date to see if there's a way for me to backfill any missing "as" props to sort this out.

@MattKevan
Copy link

I've had exactly the same problem as @brandinchiu – using Catalyst, which is based on HeadlessUI, causes Sentry to error.

It might not be the fault of Headless or Catalyst, but it would be useful to have something in the documentation to say that fragments can cause problems with reactComponentAnnotation. I wasted most of the day yesterday trying to figure out the problem until I realised there wasn't much I could do as it was caused by a dependency two layers deep. I'd even started looking for alternatives to Sentry before stumbling on a Discord comment that led me to this issue.

@chargome
Copy link
Member

chargome commented Oct 1, 2024

@MattKevan sorry to hear that you spend so much time debugging this, we'll add a note on our docs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

3 participants