maybe
maybe copied to clipboard
Upgrade Next & Migrate pages to app router
I've started the migration to the App Router https://github.com/maybe-finance/maybe/issues/50 after conversation with @Shpigford .
Here is how I am going:
- [x] Upgrade
next
andeslint-config-next
to14.1.0
because that is the most stable version right now. - [x] There have been a lot of type errors showing up across the app so I will be fixing those first (shared ss below).
- [ ] Add upgrades like Font Optimizations that are non-breaking and migrate
next/head
. - [ ] Put
use client
on pages while migrating them inside the app router folder one by one. Then use nested layouts and get rid ofgetLayout
function for the migrated pages. - [ ] Fix deprecated imports or breaking changes like unsupported
shallow routing
in app router, change in the functions exported fromuseRouter
hook of next, etc. - [ ] Try leveraging the route handlers and server actions from the app directory to ultimately get rid of server folder and tRPC (I will do this in a whole separate PR and name it as "migrate-to-app-router-v2")
Screenshot to show the errors that I am addressing in point 2:
Screenshot to show the errors that I am addressing in point 2:
Turns out this was mainly due to the devDependencies @types/react
& @types/react-dom
not being upgraded corresponding to their core packages i.e. react
& react-dom
.
I also used this opportunity to upgrade typescript package as well because it looks very stable since it has over 9m downloads compared to the version we were using that had 1.5m downloads.
@ExplorerAadi are you sure this is a good idea and aligned with the long term interests of the project? What value do we get from introducing app router and leaning more heavily into Next.js development patterns instead of building features with the existing patterns?
See my comment here for more context where I'm coming from: https://github.com/maybe-finance/maybe/commit/6f9216f41e4f165d7682867d4f234127dda93859#r137816515
I'm not just concerned about "going all in on Next.js", I'm concerned that we're moving in the wrong direction with the project (making the code less portable & investing time into work that doesn't help us acquire users).
It concerns me that this work is being prioritized over adding features or refactoring the code to make it more portable / have less vendor lock-in.
To be blunt about it: You & other Maybe team members will spend a non-trivial amount of time updating the codebase to eliminate the server
folder, and at the end the app will have no new features, or be any more portable. It will simply do things the "Next.js" way, instead of the "Express" way.
It doesn't make sense to me from a cost benefit standpoint, but perhaps I'm missing something here. I'd rather developers stay focused on building features, adding integrations, simplifying code where possible, and making it easy to deploy (via docker image for example) so we can acquire more devs and users.
It concerns me that this work is being prioritized over adding features or refactoring the code to make it more portable / have less vendor lock-in.
@gamedevsam Both Next.js Pages and App Router are 100% open source. This repo doesn't dictate where Maybe is hosted.
From your linked message:
This type of thing is exactly why I dislike new Next.js features. The entire app is client side, so this is irrelevant, no?
This isn't accurate – before making broad claims here on the App Router changes, I would recommend giving the docs a read or trying things out. Client components are still pre-rendered. They are effectively the same as components in the Pages Router.
Try leveraging the route handlers and server actions from the app directory to ultimately get rid of server folder and tRPC (I will do this in a whole separate PR and name it as "migrate-to-app-router-v2")
Agree with doing this in a separate PR. We can probably satisfy 95% of the mutations here with Server Actions, removing the extra manually written API layer (and additional tools needed to make things type safe over the network).
@gamedevsam Thanks a lot for such a detailed input, I really appreciate it. First of all, I may not be completely sure about the long term interests of the project but from the readme and conversations I have gone through so far, it seems to be offering a good DX and ability to self host the app smoothly.
On that note I don't see how this can become a problem (let's forget RSC because I want to discuss it thoroughly in another PR hence kept it separate in tasks too) because I have hosted a monorepo with three client apps (using app router) and a node server. The client apps were deployed on Vercel (but could be on aws too) and the server was on Railway and I believe the configs can be simplified to achieve that ideal one click deployment. (I would happily take a dig at it too after this).
I would also not call it just another "shiny thing" because that is the general direction I see the whole React ecosystem going in (with much larger repos like cal.com also migrating to app router incrementally). I don't see how going away from the direction of the entire community would benefit us in the long run considering the nature of frontend technologies. While saying that, I am trying to stick to the most stable releases as you can see in my task list too.
I would also never recommend going the SPA way because of how easily it bloats up and how hard you have to work on getting small performance gains instead a routing based approach which is pretty battle tested at this point and comes with so many perf gains out of the box.
On another note, I do agree with your point on having a docker file going a long way and that's why the goal with this PR was solely to work on App Router migration first. However, I might be missing something important here since you seem to have much better idea about this project from ground up and also the future direction.
Would really appreciate inputs from other core maintainers. @tmyracle @Shpigford .
Meanwhile I will be moving forward with the changes and can stop mid-way if the maintainers decide that we don't want to take this path.
@ExplorerAadi don't mistake my enthusiasm for Maybe with decision making power, or authority of any sort. As far as I know @Shpigford is the sole decision maker (he has merge power), but other than that we're all just contributors until someone else is granted merge powers. With that said I am an experienced full stack developer (this is me), and have an interest in seeing this project develop and eventually using it myself. Not sure where to contribute yet, still looking and learning (also I have a baby on the way, so that will keep me busy for a while).
Seeing @leerob lurking around gives me more confidence about sticking with Next.js long term. He's an expert with Next and has influence over how Next will evolve over time. I'm also totally open to learning new things, I've been using Next's pages router for a long time, and have not yet made the jump to App router on my side projects due to lack of time / poor return on investment for my use case.
I'm changing my tune here and will support this PR and other Next related modernization efforts. Hopefully @leerob will keep an eye on this project and help us write maintainable, efficient and portable Next.js code.
I am running into this build error a lot while running the app and can't seem to narrow down the issue. There have been very few package upgrades so not clear if this is due to anti-pattern we have or something else. Any suggestions would help.
Found a discussion around a similar error but in my case the envs are setup fine because master branch runs perfectly. https://github.com/vercel/next.js/discussions/58175
@gamedevsam Ahhh my bad but definitely agree and appreciate your thoughts and responses here, thanks a lot. Looking forward to getting your help and contributing together 🙌 And also congrats 🎊