open-xyz / informatician

Unleash an open source book platform where literature meets community, and knowledge knows no bounds.
https://informatician.in
MIT License
61 stars 136 forks source link

Migrate to Next.js 13 #659

Closed rohansx closed 1 year ago

rohansx commented 1 year ago

Checkout discussion #660

github-actions[bot] commented 1 year ago

Hello rohansx, thanks for opening a issue, your contribution is valuable to us. The maintainers will review this issue and provide feedback as soon as possible.

crocmons commented 1 year ago

Recently I learned Next.js 13 & do some projects on that...I would love to do this so that my skills will be enhanced..Can you assign this issue to me as a GSSOC23 contributor? Screenshot (442)

rohansx commented 1 year ago

@crocmons have u gone through discussion #660

crocmons commented 1 year ago

@crocmons have u gone through discussion #660

yeah..I read it properly..

rohansx commented 1 year ago

@crocmons so are u planning to migrate it in one go?

rohansx commented 1 year ago

@crocmons How long will it take for you to successfully migrate this?

crocmons commented 1 year ago

@crocmons so are u planning to migrate it in one go?

yeah, I guess I need 1 week to successfully migrate this code into next.js 13...and if I have any trouble migrating this code then I will tell you...

rohansx commented 1 year ago

Ok then, go ahead

s2sharpit commented 1 year ago

Hello there,

I'm interested in collaborating to migrate the website to Next.js. I have extensive experience in building websites with Next.js, some of which include B. S. Sen. Sec. School and my portfolio s2sharpit.me.

@rohansx @crocmons, I would be grateful for the opportunity to collaborate with you on this project. Can I join in?

Looking forward to your response!

धन्यवाद

rohansx commented 1 year ago

@s2sharpit yes you can collaborate with @crocmons if is she needs any assistance. @crocmons Do you want to collaborate with him?

s2sharpit commented 1 year ago

Hey @crocmons,

Looking forward to your response!

crocmons commented 1 year ago

@rohansx @s2sharpit Yeah, sure, why not it's a great way to learn...

rohansx commented 1 year ago

OK then you guys can work for this issue together. @s2sharpit You can take css part which needs to be migrated into tailwind css. Is that ok or something else you want to work on?

s2sharpit commented 1 year ago

Hi @crocmons,

I hope you're doing well. I wanted to reach out and let you know that I've just created a new branch called next-ts-tw-migrate for our ongoing migration process. It would be fantastic if you could assist me by creating a pull request and incorporating the code for the Next.js migration that has been completed thus far into this branch, and, If you encounter any problems or difficulties during the process, let me know.

Your expertise and collaboration would be greatly appreciated. Thank you so much for your support!

crocmons commented 1 year ago

Hi @crocmons,

I hope you're doing well. I wanted to reach out and let you know that I've just created a new branch called next-ts-tw-migrate for our ongoing migration process. It would be fantastic if you could assist me by creating a pull request and incorporating the code for the Next.js migration that has been completed thus far into this branch, and, If you encounter any problems or difficulties during the process, let me know.

Your expertise and collaboration would be greatly appreciated. Thank you so much for your support!

ok

s2sharpit commented 1 year ago

Hey @crocmons Are you facing any problems while creating the pull request?

crocmons commented 1 year ago

Actually, I need some time to work on it.. @s2sharpit I haven't created the pull request yet... @rohansx can you extend the time duration of this issue? Because I started today & there is a lot of code I have to remove also modify the code structure...

s2sharpit commented 1 year ago

Actually, I need some time to work on it.. @s2sharpit I haven't created the pull request yet... @rohansx can you extend the time duration of this issue? Because I started today & there is a lot of code I have to remove also modify the code structure...

Hey @crocmons , I completely understand that it can be a complex task, and I appreciate your efforts so far. Please don't feel rushed, as I understand that removing and modifying code, as well as setting up files for Next.js and TypeScript, requires careful consideration and effort.

Just to clarify, I'm not expecting the entire migration to be completed immediately. My suggestion is to create a pull request for the part that has been done so far, or even set up the initial Next.js and TypeScript configuration. From there, we can gradually add the rest of the code to the new branch as we make progress.

Additionally, if you're not comfortable creating the starting files for Next.js and TypeScript, I'd be more than happy to handle that part. Just let me know, and I can take care of it. We want to ensure a smooth transition, and I'm here to support you in any way I can.

I'd like to add that creating a separate branch and adding the files to it will greatly help our collaborators(like me) understand the progress made and who is working on which part. It will provide transparency and facilitate smoother collaboration within the team.

If you encounter any challenges or need any assistance during the process, feel free to reach out. Let's collaborate on this together and take the necessary time to ensure a successful migration.

Thanks again for your dedication to this project, and I look forward to working with you on the pull request.

crocmons commented 1 year ago

Actually, I need some time to work on it.. @s2sharpit I haven't created the pull request yet... @rohansx can you extend the time duration of this issue? Because I started today & there is a lot of code I have to remove also modify the code structure...

Hey, It's not to do whole migration today. It's just to create a pull request for the part you have done yet or the starting files.

Ohh yeah, I got it. Let me create then..

crocmons commented 1 year ago

wait @s2sharpit typescript.. we don't need to add typescript here? In discussion #660 not mentioned typescript. Please read it properly.. Tomorrow I'll give you the pr so that you can work on this issue..

s2sharpit commented 1 year ago

Hey @crocmons Can you tell me what error are you getting?

crocmons commented 1 year ago

yeah, @s2sharpit I'm getting errors on that branch now it was fixed ..And Also I create a pr but not completed yet still need to work on it...

s2sharpit commented 1 year ago

Hello,

I hope this message finds you well. I wanted to provide you with an update on the project's progress. So far, I have made significant headway by creating the initial files and successfully implementing the navbar.

To view the website's current state, please visit: https://informatician-next.s2sharpit.me

Furthermore, I would like to mention that @crocmons, you have the flexibility to choose to work on the landing page or any other task that aligns with your preferences and expertise. We value your contribution and want to ensure you can contribute in a way that suits you best.

If you have any questions or suggestions, please feel free to share them with us. Thank you for your continued support.

crocmons commented 1 year ago

One second how could you merge your code here...PA will be merged...and the second thing I worked on the file & folder structure in my pr according to the next 13 rules...that's why I remove the src folder and separate the component, utils folder, layout ,pages file should be inside on the app folder. @s2sharpit @rohansx

rohansx commented 1 year ago

Please don't merge and assign labels by yourself otherwise it may lead to disqualification from the gssoc program. Revert the PR and remove the label and create PR again. You should only review the PR and accept/request changes in the PR and as soon as you review the changes and accept it, I'll merge from my side.

rohansx commented 1 year ago

@s2sharpit Please co-ordinate with @crocmons with the regarding issues

s2sharpit commented 1 year ago

Dear @rohansx,

I sincerely apologize for the inconvenience caused. I understand the importance of following the correct procedure and I assure you that I will adhere to it moving forward. I will revert the pull request, remove the label, and create a new pull request once the necessary changes have been implemented.

Thank you for your understanding in this matter. I will solely focus on reviewing the pull request and providing feedback as required. After the changes have been accepted, I will patiently await your decision on the merging process.

Additionally, I acknowledge your request to coordinate with @crocmons regarding the related issues. I will reach out to @crocmons to ensure effective collaboration and resolve any outstanding matters.

Once again, I deeply apologize for any oversight and I will take extra care to strictly adhere to the program guidelines in the future.

Thank you for your understanding and guidance.

s2sharpit commented 1 year ago

Hey @crocmons Thank you for your feedback. I understand your concern about the file and folder structure, but I wanted to clarify that the src directory actually adheres to the Next.13 rules as well. Next.js provides flexibility in organizing the project structure, and the documentation on their official website (https://nextjs.org/docs/app/building-your-application/configuring/src-directory) supports this approach. Please take a look at the link for more information. If you have any specific suggestions or concerns regarding the src directory, I'm open to discussing them further. Thank you!"

crocmons commented 1 year ago

Hey @crocmons Thank you for your feedback. I understand your concern about the file and folder structure, but I wanted to clarify that the src directory actually adheres to the Next.13 rules as well. Next.js provides flexibility in organizing the project structure, and the documentation on their official website (https://nextjs.org/docs/app/building-your-application/configuring/src-directory) supports this approach. Please take a look at the link for more information. If you have any specific suggestions or concerns regarding the src directory, I'm open to discussing them further. Thank you!"

@s2sharpit I know that but according to the latest version, it's more convenient to use the app folder..Let me handle the folder structure because we have to add the API routes over there.. and also you add the component, utils folder inside the src folder which is not good for routing ..

Screenshot (470)

s2sharpit commented 1 year ago

Alright, @crocmons. Since I've already raised some pull requests, I suggest we wait for those to be merged first. Once they're merged, you can go ahead and raise the pull request for the folder structure changes.

crocmons commented 1 year ago

Also I noticed that you included global.css, a favicon inside the app folder it should be added on the styles, public folder @s2sharpit Screenshot (471)

crocmons commented 1 year ago

Alright, @crocmons. Since I've already raised some pull requests, I suggest we wait for those to be merged first. Once they're merged, you can go ahead and raise the pull request for the folder structure changes.

yeah, sure

s2sharpit commented 1 year ago

Also I noticed that you included global.css, a favicon inside the app folder it should be added on the styles, public folder @s2sharpit

Thank you for your feedback. Regarding the favicon, I placed it in the app folder because Next.js automatically detects and uses the favicon from that location. This ensures proper functionality and saves time by avoiding the need to manually configure it.

Regarding the global.css file, since we're using Tailwind CSS, a single global.css file suffices, eliminating the need for a separate styles folder.

s2sharpit commented 1 year ago

Alright, @crocmons. Since I've already raised some pull requests, I suggest we wait for those to be merged first. Once they're merged, you can go ahead and raise the pull request for the folder structure changes.

yeah, sure

You can create pull request now for file structure issue @crocmons

s2sharpit commented 1 year ago

Hey @crocmons,

As we are currently in the process of migrating the project to Next.js, a significant number of files are being changed with each pull request. Therefore, it is essential to synchronize your branch before making any pull requests to avoid unnecessary errors and conflicts.

Regarding the recent PR (#808), I wanted to bring to your attention that the theming of the project has been adversely affected. This issue arose because the pull request was merged without synchronizing the code. Consequently, the full theming has broken, resulting in an inconsistent appearance throughout the application.

Additionally, I would like to reiterate the importance of the favicon that was added to the 'app' directory. Next.js automatically uses this favicon for the website. Unfortunately, I noticed that you deleted it after the addition. It would be greatly appreciated if you could cooperate and retain the favicon to maintain consistency across the project.

To provide more context, I encountered the following errors while deploying on Vercel: image

Error got in terminal: image

I appreciate your cooperation in addressing these issues promptly. Resolving them will significantly contribute to both the stability and the successful migration of the project to Next.js.

Additionally, I am eagerly waiting for these issues to be fixed as I won't be able to proceed with my work until they are resolved. So, please prioritize fixing them as soon as possible.

s2sharpit commented 1 year ago

Link of deployment of my last pr deployment: https://informatician-git-theme-s2sharpit.vercel.app

locally rendering application after merging your pr: image

s2sharpit commented 1 year ago

Hi @rohansx, Could you please take a moment to review the ongoing issue discussed above? Your input and guidance on the matter would be invaluable.

Thank you for your attention.

s2sharpit commented 1 year ago

Hello @rohansx and @crocmons,

I wanted to let you know that I have addressed the errors and made the necessary changes for a successful deployment of the website. Additionally, I have fixed the styling of the image section, which was previously broken.

I kindly request you to review the pull request (#818) at your convenience.

crocmons commented 1 year ago

Hello @rohansx and @crocmons,

I wanted to let you know that I have addressed the errors and made the necessary changes for a successful deployment of the website. Additionally, I have fixed the styling of the image section, which was previously broken.

I kindly request you to review the pull request (#818) at your convenience.

wait I'm currently working on the home component and now added the styling on the hero section...@s2sharpit

s2sharpit commented 1 year ago

Hi @rohansx,

I hope you're doing well. I have a question regarding the assignment of levels to pull requests (PRs) in our repository. Specifically, I would like to understand the criteria and considerations that are taken into account when determining the level assigned to a PR.

I have recently submitted multiple PRs to the repository, and I noticed that each of them was labeled as Level 1. I was hoping you could shed some light on the reasoning behind this assignment. It would be helpful to know if there are specific factors, guidelines, or metrics that are used to determine the level of a PR.

Thank you in advance for your assistance. I appreciate your time and look forward to your response.

s2sharpit commented 1 year ago

Hey @rohansx I would request you to merge the all the pull requests for next-ts-tw-migrate branch as soon as possible as it will help us to work further in the migration process.

crocmons commented 1 year ago

@rohansx @SiddhantPawar03 Can you please tell me what should be the db name?

crocmons commented 1 year ago

@rohansx #936 & #944 please add a level to this pr...

Akanksha-202 commented 1 year ago

@rohansx @s2sharpit @crocmons I would love to help you both with this if you need more assistance, I have good experience in tailwind css.

crocmons commented 1 year ago

@rohansx, I think the 'next-ts-tw-migrate' branch is almost done. So, you can merge it into the main branch. We will do the rest of the implementation on the main branch so that other contributors can continue their work with this newly developed branch.

rohansx commented 1 year ago

@s2sharpit @crocmons I saw by running the code of the nextjs branch, and its design has changed significantly from the main branch, including changes to the home page and other pages as well. So, please take a look at it once by comparing with the main branch deployed link

s2sharpit commented 1 year ago

@s2sharpit @crocmons I saw by running the code of the nextjs branch, and its design has changed significantly from the main branch, including changes to the home page and other pages as well. So, please take a look at it once by comparing with the main branch deployed link

Thank you, @rohansx, for taking the time to explore the nextjs branch! Our goal was to improve the website, so we resolved any identified UI issues and made it more attractive where we felt it needs improvement. The ultimate aim was to create a seamless and enjoyable website. We would greatly appreciate your valuable feedback or any suggestions you might have for further improvement. Your input is essential to us, and we look forward to hearing from you!

rohansx commented 1 year ago

@s2sharpit @crocmons I was facing issues with the process you mentioned, so I created a copy of the main branch and named it 'v1.' Then, I copied all the files from the 'next-js-tw-migrate' branch and overwrote the files in the 'main' branch. After that, I pushed the changes to the main branch.

The migration to Next.js has been successfully completed in the main branch.

Apologies for the delays, now you can proceed with the further process and issues. Thanks for helping throughout this open source program. You guys are great contributors and collaborators. Appreciate your contributions.

rohansx commented 1 year ago

@s2sharpit @crocmons Now, there is a main task to be done, which is to integrate the open-source book reader platform with books in the format of pdf or epub. Let me know if you guys would like to take on this task.

s2sharpit commented 1 year ago

@s2sharpit @crocmons I was facing issues with the process you mentioned, so I created a copy of the main branch and named it 'v1.' Then, I copied all the files from the 'next-js-tw-migrate' branch and overwrote the files in the 'main' branch. After that, I pushed the changes to the main branch.

The migration to Next.js has been successfully completed in the main branch.

Apologies for the delays, now you can proceed with the further process and issues. Thanks for helping throughout this open source program. You guys are great contributors and collaborators. Appreciate your contributions.

Fantastic news, @rohansx! Thank you for the kind words and for all your assistance. Just a quick heads up, we'll need to update the framework preset in Vercel. Let's get that done and continue with the process. Appreciate your continued support.

Screenshot (495)