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

NextJS: Upgrade From Pages to App #702

Open
1 of 3 tasks
kyleecodes opened this issue Nov 28, 2023 · 10 comments
Open
1 of 3 tasks

NextJS: Upgrade From Pages to App #702

kyleecodes opened this issue Nov 28, 2023 · 10 comments
Assignees
Labels
complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days feature/enhancement New feature or request help wanted Extra attention is needed stale This issue or PR is inactive
Milestone

Comments

@kyleecodes
Copy link
Member

kyleecodes commented Nov 28, 2023

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

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 kyleecodes added feature/enhancement New feature or request help wanted Extra attention is needed complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days state: blocked Task is blocked. priority: soon Should be prioritized soon. labels Nov 28, 2023
@kyleecodes kyleecodes added this to the 02. Roadmaps milestone Nov 28, 2023
@kyleecodes kyleecodes removed the state: blocked Task is blocked. label Jan 2, 2024
@kyleecodes kyleecodes removed the priority: soon Should be prioritized soon. label Mar 25, 2024
@boodland
Copy link
Contributor

boodland commented Jun 17, 2024

@kyleecodes please assign it to me.

This one I would recommend to split it in several sub tasks/issues as follow:

  1. 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 Move store logic from app to store folder #1019
  2. Copy _app.tsx and _document.tsx within a root layout Copy _app.tsx and _document.tsx within the Root Layout #1040
  3. Iteratively migrate pages to app router, one task/issue per migration. A possible unordered sequence would be:

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
  1. Remove _app.tsx and _document.tsx

@boodland
Copy link
Contributor

boodland commented Jul 1, 2024

@eleanorreem or @annarhughes can you have a look to my previous comment? I would like to work on this issue. Thanks!

@kyleecodes
Copy link
Member Author

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: #918? Comment to be assigned there and we'll get you started.

@boodland
Copy link
Contributor

boodland commented Jul 3, 2024

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.

Copy link
Contributor

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. 🌸

@kyleecodes
Copy link
Member Author

@eleanorreem We want to check what your input is on this?

@eleanorreem
Copy link
Contributor

eleanorreem commented Jul 14, 2024

@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:

  1. 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)
  2. Move app/ folder content to store/ folder
  3. Copy _app.tsx and _document.tsx within a root layout
  4. Iteratively migrate pages to app router, one task/issue per migration:
  5. include the new next/head, and routing hooks - i.e. all new features should be used in each page migration
  6. 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
  7. 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
Copy link
Contributor

boodland commented Jul 15, 2024

@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:

  1. 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)
  2. Move app/ folder content to store/ folder
  3. Copy _app.tsx and _document.tsx within a root layout
  4. Iteratively migrate pages to app router, one task/issue per migration:
  5. include the new next/head, and routing hooks - i.e. all new features should be used in each page migration
  6. 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
  7. 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.

@eleanorreem
Copy link
Contributor

eleanorreem commented Jul 17, 2024

@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!

Copy link
Contributor

As per Chayn policy, after 30 days of inactivity, we will be unassigning this issue. Please comment to stay assigned.

@github-actions github-actions bot added the stale This issue or PR is inactive label Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days feature/enhancement New feature or request help wanted Extra attention is needed stale This issue or PR is inactive
Projects
Development

No branches or pull requests

3 participants