GiftForGood / website

https://www.giftforgood.io
MIT License
4 stars 5 forks source link

[RFC] Refactoring GFG repo #426

Open kohchihao opened 3 years ago

kohchihao commented 3 years ago

Issue

As GFG repo continues to grow larger, the current folder structure doesn't seem be as clear and direct to a developer.

As a result, differentiating from a dummy component from a page component is difficult.

Suggestion

src/components:

src/pages:

src/utils

src/constants

Additionally, try to group the import statements based on their groups if possible, i.e.

// components
import Header from '@components/header';
import SessionProvider from '@components/session/modules/SessionProvider';
import ContactUsPage from '@pages/contactUs';

// constants and utils
import { WISHES } from '@constants/search';
import { isAuthenticated } from '@utils/authentication/authentication';

// dynamic imports
const TopNavigationBar = dynamic(() => import('@components/navbar/modules/TopNavigationBar'), { ssr: false });
const BottomNavigation = dynamic(() => import('@components/navbar/modules/BottomNavigation'), { ssr: false });
const Footer = dynamic(() => import('@components/footer/Footer'), { ssr: false });

Template to copy

// components

// hooks

// constants and utils

// dynamic imports

Benefits

Downsides

kohchihao commented 3 years ago

Please share your concerns and suggestions if any

jamessspanggg commented 3 years ago

I agree that the above is a more maintainable structure, but refactoring this might take quite an effort so I suggest that we do this incrementally, perhaps on a page by page basis

kohchihao commented 3 years ago

@jamessspanggg Agreed. This should be done incrementally. After this is approved by everyone, I will make a todo list within this issue to track what needs to changed.

jinyingtan commented 3 years ago

Folder structure looks good with clear purpose for each location. Just a few questions:

  1. src/components is for reusuable components only right?
  2. src/pages/{pageName}/utils: Any examples of what utils we have?
  3. In this case, colors constants should be in generic constant?
kohchihao commented 3 years ago

@jinyingtan

Regarding

  1. Yes. In some sense. Stuff that can be within here could be our Cards, Buttons, Footer, etc...
  2. Could be our current utils/algolia/filteringRules --> if we can split them into respective pages, it much easier for the developer to find.
  3. Yes. Colors will be within generic constants
kohchihao commented 3 years ago

After speaking to everyone. We have decided to go ahead with this major refactoring. I will put a Todo list inside this issue to keep track of the changes that we need to make over the subsequent weeks.

kohchihao commented 3 years ago

Let's refactor pages first and leave the individual components alone.

By doing this refactor, there might be duplicated code. It is fine because it helps with the readability of the codebase overall. But we shouldn't be afraid of having some duplicated code.