shizuko-akamoto / Trecipe

CPSC 436I Project (2020S)
1 stars 3 forks source link

#47 Restructure project and add Prettier #52

Closed knox153 closed 4 years ago

knox153 commented 4 years ago

Closes #47

Sorrrry lots of changes in this PR :penguin: let me know if there is any merge conflict.

Create-react-app already setup ESLint as part of the workflow. If there is a potential error in our code, the warning is usually being printed to the terminal (IDE can be configured to show the ESLint error too). Sooo there is no need for us to set up a linter. The current rules we are using can be found here: link. Additional rules can be added if required.

But this ESLint config does not enforce any code style, and having a consistent codebase is valuable. This is where Prettier comes to the rescue:

  1. Prettier
    • It is configured to run whenever we commit a file using git hook. Now we can code in any style we want (even spaghetti code) and Prettier will automatically take care of the formatting when we commit.
    • Ran Prettier on existing code to format them
    • Added dependencies: husky, lint-staged, prettier
  2. .gitattribute
    • Prettier by default is configured to format the files with Line Feed (LF) and our repo is using LF too
    • Adding .gitattribute ensure the file we checkout is always in LF format
    • If your current local repo is in Carriage Return, Line Feed (CRLF), there will be some warning when you commit. But once the files get converted into LF after the commit, there won't be any warning in the future
  3. Reorganized project structure
    • Project is restructured based on this article
    • Components folder contains all the reusable components and any relevant files associated with these components.
    • Pages folder contains components that are only used on those pages
    • Services folder contains anything else that is not related to a component/page.

Here is a tree view of the folder structure: :point_down::point_down::point_down:

Click to expand! ``` |- |-App.scss |-App.test.tsx |-App.tsx |-components |---animation.scss |---Button |-----Button.test.tsx |-----button.test.tsx.snap |-----Button.tsx |-----buttons.scss |---fontawesome.tsx |---Footer |-----footer.scss |-----Footer.tsx |---Header |-----header.scss |-----Header.tsx |---Menu |-----menu.scss |-----Menu.tsx |---mixins.scss |---NavBar |-----navBar.scss |-----NavBar.tsx |---PhotoUploader |-----PhotoUploader.tsx |---SearchBar |-----searchBar.scss |-----SearchBar.tsx |---Toggle |-----Toggle.tsx |-----toggleSwitch.scss |---variables.scss |-index.css |-index.tsx |-pages |---MyTrecipe |-----AddPopup |-------addPopup.scss |-------AddPopup.tsx |-----CardMenu |-------CardMenu.tsx |-----Filter |-------FilterButton.tsx |-------filterButtons.scss |-------FilterSelector.tsx |-----MyTercipes.tsx |-----MyTrecipes.scss |-----TrecipeCard |-------DefaultImage.png |-------trecipeCard.scss |-------TrecipeCard.tsx |-react-app-env.d.ts |-services |---serviceWorker.ts |---setupTests.ts ```
knox153 commented 4 years ago

Could we configure prettier's --jsx-bracket-same-line to true? I think the default is false and it's a little hard to read.

Of course, updated the rule but only 5 files got modified. Let me know if there is any more rules you wanted to add.

Since we're using typescript, we should have these added to dependency as well? @typescript-eslint/parser @typescript-eslint/eslint-plugin

These two dependencies come with create-react-app too haha, it is so convenience. We can view all the rules from different packages (ESLint, react, typescript-eslint) at their repo: link

Should we also add `"eslint --fix" to lint-staged?

Done 👍

shizuko-akamoto commented 4 years ago

Could we configure prettier's --jsx-bracket-same-line to true? I think the default is false and it's a little hard to read.

Of course, updated the rule but only 5 files got modified. Let me know if there is any more rules you wanted to add.

Since we're using typescript, we should have these added to dependency as well? @typescript-eslint/parser @typescript-eslint/eslint-plugin

These two dependencies come with create-react-app too haha, it is so convenience. We can view all the rules from different packages (ESLint, react, typescript-eslint) at their repo: link

Should we also add `"eslint --fix" to lint-staged?

Done 👍

Yay! Thank you!