-
-
Notifications
You must be signed in to change notification settings - Fork 130
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
Update to React Router v7 #415
Conversation
Co-authored-by: Sergio Xalambrí <[email protected]>
Co-authored-by: Sergio Xalambrí <[email protected]>
src/server/json-hash.ts
Outdated
@@ -15,7 +15,7 @@ type ResponseResult<LoaderData> = { | |||
export async function jsonHash<LoaderData extends Record<string, unknown>>( | |||
input: LoaderData, | |||
init?: ResponseInit | number, | |||
): Promise<TypedResponse<ResponseResult<LoaderData>>> { | |||
): Promise<UNSAFE_DataWithResponseInit<ResponseResult<LoaderData>>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be left for TS to infer it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree, especially now that TypedResponse got nuked out of existence
test/react/fetcher-type.test.tsx
Outdated
import { describe, expect, test } from "vitest"; | ||
|
||
import { useFetcher } from "@remix-run/react"; | ||
import { useFetcher } from "react-router"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import can be merged with the react-router import above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I was using patterns to rename so there might be a few of these around, I think biome can also handle this on it's own if configured, but I might be wrong!
Co-authored-by: Sergio Xalambrí <[email protected]>
@sergiodxa I hope this helps you a bit to get fully there, I am not familiar with the codebase, especially the test setup, so it's a bit over my head on how to fix it and make it work, I also added some breaking changes by changing json to data, as |
@sergiodxa sorry to ask that but does this PR has everything already to be merged and shipped? I'm in middle of remix 2 -> react router 7 migration and this package is the last remaining >.< If you need any help so we can speed this up, please let me (or us) know |
… updating preload path in bunfig.toml
Hey, cool to see this coming into remix-utils. Are you planning a new major release? Right now [email protected] prevents the upgrade to RR7:
Thanks! |
I need to ensure tests pass on main before releasing it. |
Hey 👋 , do you think you align with the same peerDependencies as React Router for the React version ( |
A lot of issues with this, mainly:
Some types are not exported anymore
The tests fail a lot due to installGlobals missing
The json method has been removed and that kind of changes the dynamic of the whole test suites, and a lot of utils