refactor code

[hsk-kr]

Hi, I'm from the Discord Reactiflux channel.

First of all, I want to say, this project is awesome. I was feeling like I was using a well-made PC application.
Design and animation are remarkable.

I made some changes and it won't affect the behaviors of the app.
Since we don't have tests, I think you need to review by yourself.
I am going to share why I made these changes and some ideas.

---

• removed unnecessary brackets for string to clarify that it is pure string
  If a value doesn't include brackets, you can know it is a pure string at first glance.

• removed spacebar at the start and the end of classnames.
  I'm not sure if it is a typo or has a purpose. If it has a purpose like some js code changes className and assumes these have a space in className.

• removed unnecessary async keywords

  onClick={async () => await changeDate(+1)}

  In this case, you don't need to use async. By removing the async keyword, you can know that the function doesn't include the logic that needs the async keyword.

• removed unnecessary wrap functions in event handlers

  onClick={async () => await handleAddProject()}

  In this case, you can just attach the handleAddProject function to onClick unless the function uses arguments.

• removed unnecessary type conversion.

• fixed timeout releasing logic in DeploymentBar.
  A return function of the setTimeout function isn't used to release it.

• used a function to prevent to write the same logic in Header
  Since the three functions has the same logic, I removed the duplicate code by defining a function.

• relased timeouts when it's unmounted in ModeButtons
  It doesn't clear timeout ids, so I added the release logic and stored ids in a ref.

• improved rendering bullets logic in StorySection
  since they had the same logic, I removed the duplicate code by storing values in a state. I didn't make a change for refs.

---

Refactoring ideas

• Define functions with proper types to store and retrieve values from localStorage.

     localStorage.setItem("newsletter", "true");
  
     (localStorage.getItem(LOCALSTORAGE_THEME_KEY) as
                   | typeof LIGHT_THEME_KEY
                   | typeof DARK_THEME_KEY) || typeof LIGHT_THEME_KEY

  It cannot guarantee data consistency. It may cause wrong data insertion.

  type LevelType = ['level', number];
  
  type MessageType = ['message', string];
  
  type TokenType = ['token', Token];
  
  type StoreValueFunc<T> = T extends [string, any] ?
                         (key: T[0], value: T[1]) => void :
                         never;
  
  type StoreValue = StoreValueFunc<LevelType> &
                   StoreValueFunc<MessageType> &
                   StoreValueFunc<TokenType>;
  
  const storeValue:StoreValue = (key, value) => {
   localStorage.setItem(key, JSON.stringify(value));
  }
  
  storeValue('level', 50);
  
  storeValue('message', 'message');
  
  storeValue('token', {
   accessToken: '2YotnFZFEjr1zCsicMWpAA',
   refreshToken: '7b2d6f1e-4a0d-4e1b-a8e8-2e2c9eae1f30',
  });
  
  // ❌ type error!
  storeValue('level', 'message');
  
  type RetrieveValueFunc<T> = T extends [string, any] ?
                           (key: T[0]) => T[1] | null :
                           never;
  
  type RetrieveValue = RetrieveValueFunc<LevelType> &
                      RetrieveValueFunc<MessageType> &
                      RetrieveValueFunc<TokenType>;
  
  const retrieveValue: RetrieveValue = (key) => {
   try {
     return JSON.parse(localStorage.getItem(key) || "null");
   } catch {
     return null;
   }
  }
  
  // ✅ number | null
  const level = retrieveValue('level');
  
  // ✅ string | null
  const message = retrieveValue('message');
  
  // ✅ Token | null
  const token = retrieveValue('token');
  
  // ✅ access a field of the token
  console.log(token?.accessToken);

  I think you can ensure the types like this way, it's just an example for references.

• Used boolean variables for conditions

    userData.skills.length < 12
    // ...
    newSkill && matchingSkills.length > 0
    // ...
    localStorage.getItem("newsletter") === "true" && !emailChange

  In my point of view, it might be better to define a boolean variable with meaningful names.
  maybe like this

    isLessThanMaximumSkillNumber
    // ...
    hasMatchingSkills
    // ...
    isSubscribed

• Define functions to reduce the code duplication

  setPageData((prev) => ({
        ...prev,
        sections: prev.sections.map((section) =>
          section.type === "STORY"
            ? {
                ...section,
                details: {
                  ...section.details,
                  description_one: descriptionOneEditValue,
                },
              }
            : section
        ),
        }));
    // ...
    const target = e.target as HTMLTextAreaElement;
    target.style.height = "";
    target.style.height = `${target.scrollHeight}px`;
    e.currentTarget.select();
    // ...
    const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,4}$/;
    if (email.match(emailRegex)) {

  I have found some code is used over several components. By defining functions and calling them, you might reduce some code duplication.

• Use axios and react-query.

  when it comes to API calls, the same logic is repeated. Like adding a token, converting data to json, adding a header Content-Type, and something like that. You can reduce some code by using instances and interceptors in axios. If you use react-query, you can also take benefits of it such as error handling, and caching.

---

This is it. These are based on my experience, changes can seem not right.
I have learned a lot from your project. I never tried to create this kind of a big project.
I'm really impressed by your skills, commitment, and more.
I literally got motivated. I am thinking about starting my project too.
Anyway, thank you for your time and I will be looking forward to your opinions as well.
Have a nice day!

[hsk-kr]

The command npm run lint displays 'ESLint found too many warnings (maximum: 0).'. Maybe it's why the PR Build failed. But they were already there though..

[noahgsolomon]

thanks for the pr! Join the discord if you would like to add some more things here and there, or if u just want to keep up to date ;) https://codefoli.com/discord