Refactor Sessions Summary

[rossneilson]

Our refactors sessions were a great success and we got a lot of work done going over these

This is the PR we can use as a reference for expected changes.
#822
It looks like a lot but most changes are moving files and yarn lock, make sure you take a read through and understand the structure and definition of done
And if there is anything you think we should improve then let us know

Definition of done

• Move components into new structure (wrappers folder)
• Split up into multiple components in same file
• Add new components to their own component folder
• Define types in types/common for Components
• Completely remove styles from wrappers and logic from components
• Ensure absolute imports
• Add i18n
• Use the new wrapper as an import to replace old-components and test it works as expected and the same as current v1.1-beta branch
• Unit test
  For unit testing since we are aiming for no business logic inside the components we should be fine to do simple snapshot tests like in this PR. If you need to mock data fetching then probably you need to split things up more. For the wrappers we are also aiming not to need to test these as the bulk of logic should be in hooks which are also tested (I believe we have some examples of this coming soon)
• Storybook
• Remove old-components folders

This doesn't need to be the end of these mobbing sessions, we can keep them up, just let us know in the chat when there is anything you are not sure or want feedback on refactoring.

We don't yet have storybook, we will add an example at some point soon

[rossneilson]

A summary of what we covered in steps:

Session 1

• Added new structure in temporary storybook folder
• Added storybook
• Begun splitting up proposal card into more components
• Tried to improve useParams

Session 2

• Decided to make custom hook wrapper for useParams types
• Further split up components
• Decided our approach to fewer primitives (or making them afterwards)
• Added type definition and structure

Session 3

• Went over testing and i18n (none here)

Session 4 (Just did on my own)

• Completed rest of testing
• Got working due to dependency issues on react router (NEVER import 'react-router')
• Refactored ProposalStatus (missed some hooks we will refactor later) as it was requesting data and breaking tests

[rossneilson]

Initial Refactor Issues:
#847
#848
#849
#850
#851
#852
#853
#854
#855
#856
#857
#858
#859
#860
#861
#862
#863

This covers the majority of current guild component that we will split up into their own wrappers and components. The title refers to the current folder of the component inside the old-components/Guilds/ folder currently on v1.1-beta
Once we cover all of this we will have another session to finish off the refactor, delete unused code and plan the next refactor involving more of a dxvote refactor to implement this UI.

[rossneilson]

We have a few things to clarify and define some rules regarding refactors. JS is of course very opinionated, none are wrong, we just need to define rules.
Trying to get a healthy mix of small understandable, intuitive files but not make things nested or verbose.

The first is how we want to nest smaller components in a component folder. This is one approach we merged for ActionModal:
/Components/ModalX/Components/ComponentYofX/

• First question is do we want an additional Component folder for all sub components
• Then, do we want individual folders
• And finally should we use an index.ts file that just exports the actual component for nicer imports but more files?

Next, how should we split up styles?
We can keep them in the component file, the issue is that if it gets big it can be quite annoying to scroll through styled components. You could also argue though that if it is that large then it should be refactored some more.
Or we could have one styled file in the component's directory.
Or we go further and have individual files for every single sub component.

[MiltonTulli]

All of this is very opinionated so this is just my opinion on this regarding readability and maintainability for react components.

• I think we don't need the additional components/ folder to wrap sub-components when we just have 1 or 2 sub-components. If we think about the cases that have a lot of sub-components I think it can be valid.
• I like to have individual folders for each component since each should have its own .styled, .test, .stories, etc files.
• Also, I like index files only for exports. Results in cleaner imports. At the end it is nice to be able to just import { ComponentA, ComponentB } from 'components' instead of import ComponentA from 'components/ComponentA/ComponentA'; import ComponentB from 'components/ComponentB/ComponentB' Also I prefer named exports in index files

[dcrescimbeni]

In general, I advocate for smaller files and more abstraction. What struck me the most while reading "Refactoring" was the idea of files and functions having a small but clear purpose and the ability to dig into the logic or specifics only when/if needed.

• For smaller components, I agree that file separation would be enough.
• For larger components, I'd separate them in only one folder (maybe "components"), so they don't clutter the view of the main component, meanwhile having only one .styled, .test, .story and .types in the root folder of the component. This is advantageous because:
  • encourages styles and types reusability
  • declutters the component, extracting the presentational aspect and the prop types, leaving only the organizational and logic elements
  • organizes all tests related to the component in one file. We can split the logical sections of tests using the describe block
• I'd argue that if we need a separate .styled, .test, and .story for a sub-component, we may need to abstract that component in its own common component.
• Piggybacking what Milton said, adding an index file in the component folder allows us to have cleaner imports.

[rossneilson]

That's something I didn't think about, quite like the approach of a components folder for sub components with no more folders after that. Think I like what Dino is describing as a good balance.

[rossneilson]

Main conclusion we came to for now at least is to go with the structure in ActionsModal as described by Milton. Instead of describing it in detail here I think it makes most sense to just refer to the source code. It's a very understandable intuitive structure. We will have root component files, styles, fixtures and stories. Then a components folder with a root index file and individual files for each sub-component each with its own styles and index file.
This should allow us to scale well without things getting messy, it can look verbose right now on first inspection but let's see how it goes and if need be we can change in future.
Milton will also update already refactored components to this structure.