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

feat(nextjs): app router support #1071

Merged
merged 7 commits into from
Jun 2, 2023
Merged

Conversation

subzero10
Copy link
Member

@subzero10 subzero10 commented May 19, 2023

Closes: #1070

Status

READY

Description

Adds support for the new App Router in Next.js.

Todos

  • Implement support for app router
  • Make sure that source maps are correctly uploaded for the app directory
  • BONUS: support projects built with .tsx (Typescript) or .js files

@subzero10 subzero10 added the nextjs @honeybadger-io/nextjs label May 19, 2023
@subzero10 subzero10 self-assigned this May 19, 2023
@subzero10
Copy link
Member Author

subzero10 commented May 19, 2023

I also have an example project that I will push into the nextjs package with #1072.

@subzero10
Copy link
Member Author

subzero10 commented May 24, 2023

Error reported automatically while rendering client-side component. Source maps applied successfully.

image

@subzero10
Copy link
Member Author

Error reported automatically on the server side, during a data fetching method. However, most of the context is missing because Next.js does not propagate server errors to the Honeybadger React Error Boundary (since it's on the client side).

We can look into improving this with #1055.

image

@subzero10
Copy link
Member Author

@BethanyBerkowitz Can I please get another round of review 🙏 ?

Thanks!

@BethanyBerkowitz
Copy link
Contributor

Error reported automatically while rendering client-side component. Source maps applied successfully.

image

This shows that the sourcemap was applied, but the file names are still minified... shouldn't they be the original file names and not minified ones?

Copy link
Contributor

@BethanyBerkowitz BethanyBerkowitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me, I left minor questions / comments. Did you want me to test this in some way?

packages/nextjs/src/copy-config-files.ts Show resolved Hide resolved
@subzero10
Copy link
Member Author

This shows that the sourcemap was applied, but the file names are still minified... shouldn't they be the original file names and not minified ones?

Are you referring to react-dom.production.min.js? This is a dependency, i.e. not part of the application code.
page.tsx is application code and appears to be shown correctly.

@subzero10
Copy link
Member Author

Did you want me to test this in some way?

I don't think it's necessary at this point, but if you want to play with Next.js, feel free to do it!

@subzero10
Copy link
Member Author

@BethanyBerkowitz Can you please approve, if you are happy with my replies 😄 ?

@subzero10 subzero10 merged commit e3752f4 into master Jun 2, 2023
@subzero10 subzero10 deleted the nextjs_app_router_support branch June 2, 2023 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nextjs @honeybadger-io/nextjs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for App Router
2 participants