bloom-frontend
bloom-frontend copied to clipboard
NextJS: Upgrade From Pages to App
Overview
NextJS Apps is the upgraded version of NextJS Pages. We should leverage the additional features and benefits by upgrading Bloom's frontend to NextJS Apps.
Benefits of NextJS Apps include but not limited to: enhanced scalability from improved modular approaches, advanced routing capabilities, improved performance in server-side rendering and static site generation, built-in optimizations for high traffic.
Action Items
- [x] Upgrade to Node 18: https://github.com/chaynHQ/bloom-frontend/issues/703 to unblock
- [ ] Follow this guide on upgrading to new features, such as the improved Image and Link Components: NextJS Apps: New Feature Guide.
- [ ] Follow this guide on incrementally migrating from pages to app directory: NextJS Apps: Migrating from pages to app Guide.
Note: Please document the changes to be made with the upgrades. If there are many or highly complex changes, we will open new issues for them.
Resources/Instructions
@kyleecodes please assign it to me.
This one I would recommend to split it in several sub tasks/issues as follow:
- Move app/ folder content to store/ folder (the app router uses app/ folder as convention to have the pages and layouts, I think moving any other logic outside is recommended as drives to a cleaner and more focused structure #1019
- Copy _app.tsx and _document.tsx within a root layout #1040
- Iteratively migrate pages to app router, one task/issue per migration. A possible unordered sequence would be:
- meet the team #1040
- account
- admin
- api
- auth
- courses
- partner-admin
- partnership
- subscription
- therapy
- welcome #1075
- error pages
- rest of root pages within pages/ folder
For each migration, ideally I would include:
- The migration from pages to app router following the recommended approach
- An E2E test if not available to cover the happy path of the feature/page
- A proposal of a nested layout and/or folder structure, migration to server component, next/head, routing hooks, data fetchting methods and styling among others to discuss and tackle in a follow up issue
- Remove _app.tsx and _document.tsx
@eleanorreem or @annarhughes can you have a look to my previous comment? I would like to work on this issue. Thanks!
Hey @boodland thank you for your patience. These suggestions are helpful, thank you.
Since this is a more ambitious contribution, we recommend starting with a beginner issue first to familiarize yourself. Would you be open to being assigned this issue to start: https://github.com/chaynHQ/bloom-frontend/issues/918? Comment to be assigned there and we'll get you started.
Hi @kyleecodes, sure thing, I will be delighted to work on #918, but please keep me into account for this one if possible as I would love to do it.
Thank you @boodland you have been assigned this issue! Please follow the directions in our Contributing Guide. We look forward to reviewing your pull request shortly ✨
Support Chayn's mission? ⭐ Please star this repo to help us find more contributors like you! Learn more about Chayn here and explore our projects. 🌸
@eleanorreem We want to check what your input is on this?
@boodland sorry for the delay and thank you for your patience. I was speaking with one of my team mates @annarhughes who has previously done this migration. They suggested these points:
- Follow this guide on upgrading to new features, such as the improved Image and Link Components: NextJS Apps: New Feature Guide. (as in the original issue description)
- Move app/ folder content to store/ folder
- Copy _app.tsx and _document.tsx within a root layout
- Iteratively migrate pages to app router, one task/issue per migration:
- include the new next/head, and routing hooks - i.e. all new features should be used in each page migration
- exclude data fetching and styling for now (suggestions/proposals for implementations welcome). Existing redux (RTK & RTK query) and MUI packages are deeply rooted and used throughout the app, easy improvements towards server components would be great, however major changes like removing RTK/MUI might not be worth the performance gains for us
- Remove _app.tsx and _document.tsx
Otherwise, all good to go. @boodland do you need me to create an issue for each step that you take? or are you happy to write up an issue for each page you migrate/ step that you take?
Thanks again for your input.
@boodland sorry for the delay and thank you for your patience. I was speaking with one of my team mates @annarhughes who has previously done this migration. They suggested these points:
- Follow this guide on upgrading to new features, such as the improved Image and Link Components: NextJS Apps: New Feature Guide. (as in the original issue description)
- Move app/ folder content to store/ folder
- Copy _app.tsx and _document.tsx within a root layout
- Iteratively migrate pages to app router, one task/issue per migration:
- include the new next/head, and routing hooks - i.e. all new features should be used in each page migration
- exclude data fetching and styling for now (suggestions/proposals for implementations welcome). Existing redux (RTK & RTK query) and MUI packages are deeply rooted and used throughout the app, easy improvements towards server components would be great, however major changes like removing RTK/MUI might not be worth the performance gains for us
- Remove _app.tsx and _document.tsx
Otherwise, all good to go. @boodland do you need me to create an issue for each step that you take? or are you happy to write up an issue for each page you migrate/ step that you take?
Thanks again for your input.
@eleanorreem and @annarhughes thanks for your feedback!
I have been looking at point 1 and seems to me that the new Image component is already used as there are no references to the /next/legacy/image import, so all the imports are using the new one. For the link one I have tracked where it is used and is wrapped within the custom common/Link component which doesn't seem to pass an anchor element as child which is the change that can be avoid using the new Link component from Next.js 13. although I cannot say it 100% as it is referenced from many places but the ones I have checked are just using the Link component as value for the component property in other components, so I am going to assume point 1 is fine but keep an eye when doing the migration of the individual pages in case something can be changed related to them.
With respect to point 5 migrating to new next/head and routing hooks I will include them in each page migration if possible, not sure if can be easily added as we will be creating a client page component to maintain consistency with pages approach and importing it within the page server component of the app route but I would say it should be fine to change both during the page migration.
I will add comments about point 6, as proposed, in the follow up issue of how to approach the migration to server component if possible.
@eleanorreem No need to create new issues for each sub item/issue, I have already done myself for the relocation of the store logic #1019 and I can do it for the rest that I will be working on but if you want to create them so they are all visible, please go ahead
Let me know if there is anything I misunderstood.
@boodland your comments makes sense for me. I haven't done the migration before and @annarhughes is off for a bit so go ahead and we can take it a ticket at a time!
As per Chayn policy, after 30 days of inactivity, we will be unassigning this issue. Please comment to stay assigned.