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

Next.js Rewrite #3104

Open
bdbch opened this issue Jun 15, 2023 · 8 comments
Open

Next.js Rewrite #3104

bdbch opened this issue Jun 15, 2023 · 8 comments

Comments

@bdbch
Copy link
Member

bdbch commented Jun 15, 2023

Hey @howardchung - this is quite a big one and I'd love to discuss what you think about it.

This project is aging a bit - I realized this when I did #3103 and I realized:

  1. We mix a lot of js and ts
  2. We still use CRA which is even discouraged nowadays by the core team
  3. We don't really optimize anything on the page (which we can improve a lot)
  4. The code base grew over the time and got a bit messy
  5. We are stuck on a very old version of MaterialUI.

Here is my proposal on what we could do:

  1. Start a rewrite based on Next.js
  2. Use Tailwind + shadcn to stay flexible with customizable UI elements (I'm a big fan of headless frameworks / libraries)
  3. Use the same API calls we use on the old page

I started to work on a proof on concept. I'll add a few screenshots as my current Vercel demo does not work correctly because of rejections by the API.

Screenshots of current WIP state

Home

Bildschirmfoto am 2023-06-16 um 01 29 44-fullpage


Matches View (Pro)

Bildschirmfoto am 2023-06-16 um 01 30 19-fullpage


API Page

Bildschirmfoto am 2023-06-16 um 01 30 59-fullpage


Subscribe Page

Bildschirmfoto am 2023-06-16 um 01 31 15-fullpage


Changelog Page

Bildschirmfoto am 2023-06-16 um 01 31 27-fullpage


Hero Page (currently static and demo content)

Bildschirmfoto am 2023-06-16 um 02 03 21-fullpage
Bildschirmfoto am 2023-06-16 um 02 03 24-fullpage

As you can see I started to work with the static pages first to test Next.js's static site generation and it worked fine. Now I'm currently working on the dynamic pages starting with both match overviews.

I'd love to hear what you think about this proposal.

You can check the demo here:
https://opendota-web.vercel.app/

@howardchung
Copy link
Member

Thanks for taking the initiative on this!

I do agree we are working with some really legacy code here.

I would love us to get onto 100% TypeScript--obviously that's a big effort but I think it would be great for developer productivity.

Regarding CRA--I wasn't aware it was no longer recommended and am still using it actively for new projects where it works pretty well (these are much less code than OpenDota though). I would say that if it's proving an impediment to developing due to slow builds etc., then it's worth doing the migration. If it's just "migrate to the newest shiny thing" then we may want to reevaluate because we want whatever we end up using to last 5-7 years like CRA did.

CSS: I'd like to get rid of styled-components in favor of anything that uses actual css files--don't have strong opinions on what framework that should be.

Anyway, those are my thoughts but I don't contribute a ton of new features to this project anymore--would love to get input from those who are actively working on feature development.

cc @blukai (who built our original React UI) and @albertcui

@bdbch
Copy link
Member Author

bdbch commented Jun 17, 2023

I continued to work on my prototype for now and it's really nice to work with.

What is your opinion on Tailwind? Some hate it - some love it. I really enjoy it because you can quickly draft UI elements out of it + it works really great with headless UI solutions like RadixUI (which is used by ShadcnUI)

https://www.radix-ui.com/

If you want to I could push my current state of the app on a public repo in this org so @blukai and @albertcui can take a look at it?

@howardchung
Copy link
Member

I mostly have experience with opinionated CSS frameworks like Bootstrap, Material UI, Semantic UI. But if the trend is now to adopt more modular solutions I'm happy to go along with it.

If your development version is based off the OpenDota code then maybe you could just make a PR? But if it's a separate repo perhaps you can just publish it to your own account for now

@blukai
Copy link
Member

blukai commented Jun 26, 2023

This is interesting to see 👀

I have not been following the project for a long time, but looking at current state of contributor activity I don't think that rewrite is the correct approach because it is not something that can be done for a project this large in a reasonable amount of time by a single individual.
What about a group of enthusiastic people? Sure!
Go for rewrite if you can gather a group of people who would be willing to commit.
More people may join in after new project will be initiated.


It is possible to incrementally adopt next.js into a spa codebase without initiating a rewrite.
You would want to start by making the app render in a browser like it currently does, like a regular spa without using any next.js's features like ssr, rsc, etc.
'use client' directive (if using app directory) + NoSsr wrappers(example) are one of best friends for that.
And then start making things work in rsc' or with ssr.


radix-ui is amazing.
shadcnui is hyped for some reason, but it seems very raw and immature.
About tailwind I don't have complete enough opinion.

@mehrdadrafiee
Copy link
Contributor

I like the idea of a re-write, however this could be done in parallel and in an incrementally fashion. Basically the old and the new web app would co-exist and in the old one we link to the new pages as they are being created. This will be done for all the pages/routes until the whole app is out there and then we can sunset the old one.

The good thing is that we can use TypeScript, Shadcn and any other goodies from the ground up!

All that said, I'd be happy to help re-write this project. :)

@mehrdadrafiee
Copy link
Contributor

mehrdadrafiee commented Oct 26, 2023

any updates on this? @bdbch wanna team up and work on it? :)

@howardchung
Copy link
Member

I've recently updated the build system to use Vite, which is a good first step for better dev experience.

But rather than rewriting the whole app using Next.js I'd recommend doing the following incremental changes:

  • Gradually convert all our .js(x) to .ts(x) (just adding any types and ts-ignore where needed)
  • Get rid of our proptypes code since TS will enforce this
  • With the additional type safety provided--replace styled-components with a more modern css solution
  • Figure out how to update to recent material-ui (or replace with new styling framework)
  • Replace redux. . . maybe. I don't think we derived a ton of benefit from the state management and we can probably simplify the code quite a bit by just using React Context with regular old fetch requests

@mehrdadrafiee
Copy link
Contributor

@howardchung thanks for the feedback. The steps provided seem reasonable. Let's just convert anything we can to TS and then we'll figure out the css/material-ui/other css framework part and last but not least, the redux stuff.

Here is a PR for converting the Home page to use TypeScript. #3124

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants