the-hideout / tarkov-dev

The official site for tarkov.dev - A web app to track item prices, view trader barters, quests, maps, and much more!
https://tarkov.dev
MIT License
130 stars 48 forks source link

Example Of Refactoring To Make Files More Descriptive and use jsx instead of js #929

Open DysektAI opened 2 months ago

DysektAI commented 2 months ago

1.) There are a LOT of "index.js" files being used to define components as I never seen this before in my time of using react unless using index.js to import multiple components into one file for easier use and access here is the best resource I found that explains this:

  1. Don’t use index.js as an entry point to a directory When you use index.js as an entry point to a directory, it can be difficult for other developers to understand the purpose of the file and how it fits into the overall project structure. This is because index.js files are often used as generic catch-all files that contain code from multiple components or modules.

Instead, try to name your files according to their purpose. For example, if you have a component called “MyComponent”, then name the file “MyComponent.js” instead of “index.js”. This will make it easier for other developers to quickly identify what each file does and where it belongs in the project structure. Source: https://climbtheladder.com/10-react-naming-conventions-best-practices/

2.) Properly use .js and .jsx as explained below which currently is mostly .js extensions in the project. (The reason why this isn't an apparent issue when using CRA is because it uses webpack and babel under the hood to handle these files no differently)

Vite's official documentation and community discussions provide insights on how Vite handles .jsx files differently from .js files, emphasizing the importance of using the correct file extensions for proper processing and optimization. This helps ensure that the JSX syntax is correctly transformed during the build process. Reference: https://github.com/vitejs/vite/discussions/3448#discussioncomment-749919

Shebuka commented 2 months ago
  1. Background: historically it's (was?) a CRA convention. In the code when importing you reference components by their folder name, never by the js file, and then i_dont_know_who_specificaly will look for the index.js file inside and the exported parts in it. Referencing imports by the js file is a relatively new thing required by ECMAScript. Because we recently migrated to ECMAScript (https://github.com/the-hideout/tarkov-dev/commit/784a6c1881f58a2427d6828bb9644db02fe74f3c it was pain 🤪) it can be useful to also migrate the naming convention 👍 Discussion points:
    • Should the folder be renamed too or the file named as folder?
    • Do we need a folder if we only have the jsx file inside (without CSS)?
  2. Background: historically it's (was?) a CRA convention. Totaly should be done moving forward 👍
DysektAI commented 2 months ago
  1. Background: historically it's (was?) a CRA convention. In the code when importing you reference components by their folder name, never by the js file, and then i_dont_know_who_specificaly will look for the index.js file inside and the exported parts in it.

I wasn't familiar with this older CRA convention either. Understanding this helps clarify why it was done that way.


Referencing imports by the js file is a relatively new thing required by ECMAScript.

I've heard about the need for explicit extensions with ECMAScript, but haven't needed to implement it in my project, which still runs smoothly without specifying .js or .jsx extensions. (I did have issues when using package.json type: module removing it fixed a lot of the issues but to use Vite it was required and still don't see any issues maybe my ESLint isn't configured correctly)


Because we recently migrated to ECMAScript it can be useful to also migrate the naming convention 👍 Discussion points:

  • Should the folder be renamed too or the file named as folder?

I typically organize components like 'settings' under src/components/settings/ and then import them into src/pages/settings.jsx for rendering. This setup works well for my /settings route as I can add and remove components easily or simply edit those components in their respective files if needed.


  • Do we need a folder if we only have the jsx file inside (without CSS)?

For components using inline styling, especially with frameworks like MUI that use the sx prop, external CSS files aren't necessary. However, for those that do have associated CSS, structuring folders like this could be more organized:

/src
  /components
    /BarterTooltip
      BarterTooltip.jsx
  /styles
    /BarterTooltip
      BarterTooltip.css

  1. Background: historically it's (was?) a CRA convention. Totaly should be done moving forward 👍

Once the above are addressed at least for the most part, it would be beneficial to move forward with implementing Vite.

Switching to Vite would streamline workflow, significantly reduce server startup times and improve build speeds with efficient Hot Module Reloading (HMR).

Overall, transitioning to a clearer structure and simplifying the codebase will enhance the development process greatly.

Shebuka commented 1 month ago

@DysektAI I'll say that if you want to take on the monumental effort to rename all react components from js to jsx go for it 👍

This will prepare us better for when we are ready to switch to Vite or Next.js