Codebase Review

[enesozturk]

Hi, as a new coming to the project, I saw things that I think fixing them would help us in the long run. I wanted to start a discussion on some topics to hear your thoughts. These might be the things that you possibly considered already. If it is, I would love to help on these make the codebase cleaner.

Separating business logic and views

Generally separating the component as much as small pieces and business logics is a good practice. In this way, even the big components will be managed very easily. There will be many other features or refactors for this app in the future, I see many components putting all the things together. After a while, no doubt that they will be hard to refactor/maintain, hard to understand, as a result, it will be extra effort to improve them or fix bugs (or vise versa, this might cause bugs).

Sample Solution: There can be a style guide for components structure (folder strc, naming convention, etc.). The components from now on follows this guide.

States

These code samples only exist in the next-react repo which is a legacy project, the new codebase doesn't include such usages.

In some places, there are lots of state creation and state updates sequentially. Here is an example:

Details

CommentsSection.js

const [comments, setComments] = useState([]);
const [commentText, setCommentText] = useState(“”);
const [parentComment, setParentComment] = useState(null);
const [siblingComment, setSiblingComment] = useState(null);
const [replyActive, setReplyActive] = useState(false);
const [loadingComments, setLoadingComments] = useState(true);
const [hasMoreComments, setHasMoreComments] = useState(false);
...
const myFunc = ()=>{
   setComments()
   setCommentText()
   setParentComment()
   ...
}

With React 18, this issue is fixed that React will combine all before updating the UI, if there are sequential state updates, it will make one render. But until now, it was a drawback. I think for this kind of state, we can use useReducer to store all the data together and dispatch to change more than one value.

Sample Solution: If we think there will be many states like these, we can consider using userReducer.

Constants

In many places, we use hardcoded values for endpoints, source links, color values, contract addresses, etc. I saw many big projects generally prefer bringing related constants together in a commonplace and using them in different places with safe. Because if I change one of them (ex. v1/user/settings to v2/user/settings) instead of changing the values in every component, I can just change it from where it is defined.

Here is an example I created for my last feature: packages/app/constants/endpoints.ts
Another one: packages/app/constants/modal.ts

Think of we added many other requests here;
This was just for the endpoints. There are lots for things which can be located in /constants folder.

Sample Solution: We can define constants at the root and they can be used in different places with safe.

[enesozturk]

When accessing the properties and want to sure if they are not undefined. Optional Property Access operator can be used;

if(
 myInfoData &&
 myInfoData.data &&
 myInfoData.data.profile &&
 myInfoData.data.profile.notifications_last_opened
)

to

if(myInfoData?.data?.profile?.notifications_last_opened)