-
Notifications
You must be signed in to change notification settings - Fork 928
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
App router migration #535
App router migration #535
Conversation
@carletex Let me know if any changes needs to be done. |
Hey @Anurag-Raut , thanks for this!! A few things:
|
Update :Merged the latest changes from the Next Steps :Documenting folder structure followed: But if someone has second thoughts please feel free to put up 🙌 Thinking about Barrel files at least inside Will create a nice explainer issue and we can discuss about it there Clean up: After we get 1. and 2. cleared I think we could clean up a bit and lol had some nitpicks in mind but didn't want to add those in this PR since its already too long and works great 🙌 TYSM All, We could probably merge this next week 🚀🚀 |
Awesome! Themes were broken again, fixed 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.
Themes were broken again, fixed it
Ohh thanks 🙌, missed it that we need to use custom useDarkMode
while merging
Tested it throughly and works great for me !! : https://next14-se2.vercel.app/test
Here is test commit for above link https://github.com/technophile-04/se-2/commit/d01d88d0115cbf5a5f633e0cbff72c6541200fb8 if someone wants to try
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.
🙌
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.
Tested it and works great to me. Updated a small nitpick on the route we showing in /debug page.
GREAT JOB Shiv!! 🙌
I am just trying to resolve the conflicts in yarn.lock, here are commands which I executed but for some reason the
2nd fails to check the reason I did |
I'm getting the exact same error as you :-( |
Just fixed the conflict at 5a6a6d9, the way I did this was :
SInce |
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've been testing and everything is looking good.
Thanks @technophile-04, @Anurag-Raut and everyone for reviewing and testing. This is a big one!!
Let's include this in the CLI soon.
Tysm all !!! Merging this 🚀🚀 |
Description
Migrated from Page router to new Approuter .
upgraded to Next v13.5.2 since the app router is stable from v13.4 +
Additional Information
Related Issues
_Closes #526 _
Your ENS/address: 0x72aA8A79D02705444CC6918A6766Be03AD4C29F5