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

V2 - React Router v7 #18

Merged
merged 18 commits into from
Nov 25, 2024
Merged

V2 - React Router v7 #18

merged 18 commits into from
Nov 25, 2024

Conversation

rphlmr
Copy link
Owner

@rphlmr rphlmr commented Nov 23, 2024

V2 Release for React Router 7

Description

This is a "soft" breaking change: @remix-run dependency becomes react-router.

No API changes.

Copy link

pkg-pr-new bot commented Nov 23, 2024

Open in Stackblitz

npm i https://pkg.pr.new/rphlmr/react-router-hono-server@18

commit: b9caf4c

@rphlmr
Copy link
Owner Author

rphlmr commented Nov 23, 2024

waiting for sergiodxa/remix-hono#297

@firxworx
Copy link

Hi @rphlmr thank-you for react-router-hono-server! It answered some questions I had. I have worked with react-router a ton of times on SPA's but not much with remix. Earlier today I upgraded an SPA to React Router v7 today with all future flags enabled and it was painless so I figured it was a good time to learn a little more about the feasibility to use v7 in a new full stack project.

Suggestion to unblock and improve maintainability

I thought I'd share a first thought I had when I reviewed the code in this package and remix-hono, before I even looked at the issues and saw this one:

"if react-router-hono-server is only using remix-hono/handler, and that's only a handful of lines of code, why wouldn't the author directly incorporate those crucial lines directly in react-router-hono-server?

Reference: https://github.com/sergiodxa/remix-hono/blob/main/src/handler.ts

I think adding yet another set of third-party dependencies, maintainers, etc. adds unnecessary risks for what amount to a few lines of code, especially where those lines are so critical for this package to succeed in its purpose. I think that this early PR being blocked is an early warning about future challenges to come.

The collection of exports in remix-hono while insightful are all very succinct and ripe to copy as needed.

By all means I agree to give credit where credit is due for remix-hono and sergiodxa in the README, along with mention of some of the other exports that might potentially be useful to some :) Thanks again for creating this repo :)

@rphlmr
Copy link
Owner Author

rphlmr commented Nov 24, 2024

Thank @firxworx!
You are right, it could be simpler to fork this part since it is really a basic binding and the core part of this library.
I'm also a maintainer of remix-hono and I started to re-export it. Then I realized that it would be better to compound them instead of mixing them, so I rolled back.

Let's do this, of course, with credits.

@rphlmr
Copy link
Owner Author

rphlmr commented Nov 24, 2024

Okay, there is an issue with the virtual module. It seems it only works for local code and not a library. Node resolves it before Vite or something like that.
I will try later on monday and or rollback some lines

@rphlmr
Copy link
Owner Author

rphlmr commented Nov 25, 2024

Okay, let's try a new approach: a Vite plugin!

@rphlmr rphlmr merged commit e5eed4f into main Nov 25, 2024
4 of 5 checks passed
@rphlmr rphlmr deleted the v2 branch November 25, 2024 18:31
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.

2 participants