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/migrate to react router 7 #897

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

DennisKraaijeveld
Copy link
Contributor

@DennisKraaijeveld DennisKraaijeveld commented Dec 31, 2024

Made the first changes to go from RemixV2 to React Router 7. These are only the basic changes to be made and I'm working on the dev and prod server, but they don't seem to work out of the box.

Sentry did not work so this has been updated to use different packages provided by them. We do miss some of the current integrations though.

On a different project I do got the dev and prod server working, but I had to change quite some code. I'll test this through and see if this is really necessary or not.

@hakimLyon I did start from your PR, created a new one though.

@DennisKraaijeveld
Copy link
Contributor Author

DennisKraaijeveld commented Dec 31, 2024

Some comments to be made:

  • The Sentry package has changed from the Remix one to @sentry/react. This setup still needs some testing and see if we are still receiving the correct information. For example the captureRemixServerException has no real replacement. Client and server errors are both using Sentry.captureException Not sure how this will turn out.

  • There is still a @remix-run/server-runtime dependency in some of the packages like remix-auth-* and the nasa-gcn/remix-seo package. Haven't looked into it but I had to install this dep in order to get everything working. This is causing one new typeerror within the sitemap route and I guess those packages need an upgrade so it works fine with RR7

@kentcdodds

@DennisKraaijeveld
Copy link
Contributor Author

Looks like the gcn SEO package is not being actively maintained. We could switch to: https://www.npmjs.com/package/@forge42/seo-tools

@stephen776
Copy link
Contributor

https://docs.sentry.io/platforms/javascript/guides/react-router/

Might be helpful for the Sentry stuff. I haven't tried myself yet though.

@DennisKraaijeveld
Copy link
Contributor Author

@stephen776 Thanks! Looks like they updated the docs a bit. I'll check it out soon and see where we can update

@DennisKraaijeveld
Copy link
Contributor Author

DennisKraaijeveld commented Jan 3, 2025

Looks like framework mode is not yet supported thus we have to wait until Sentry is supporting this

It does look like it can take a while (speaking about months) before a dedicated version is released: getsentry/sentry-javascript#14519

Following the React/Node integration from the link above will get you close enough for now.

@DennisKraaijeveld
Copy link
Contributor Author

Saw that the httpIntegration and prismaIntegration are exported from the @sentry/node package so we can use these now. Looks like we are all set on the Sentry part. Whenever the dedicated RR package will be released we could move to that one.

@kentcdodds
Copy link
Member

Amazing effort here! Thank you so much for doing this.

It's been a crazy few months, but I'm finally going to start getting back to work next week and I'll get to this (probably live on stream!!). I love seeing the green CI checks 💚

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, Rather than defining SerializeFrom, it is possible to directly utilize the types from the .react-router folder as follows:

import { type Route, type Info } from './+types/profile.connections.ts
...

export async function loader({ request }: Route.LoaderArgs) {
...
export const headers: Route.HeadersFunction = ({ loaderHeaders }) => {
...

export async function action({ request }: Route.ActionArgs) {
...

export default function Connections({ loaderData }: Route.ComponentProps) {
...
function Connection({
	connection,
	canDelete,
}: {
	connection: Info['loaderData']['connections'][number]
	canDelete: boolean
}) {
...

';

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

Successfully merging this pull request may close these issues.

4 participants