Safe-react: Discuss: New project structure with split ui/logic

[mikheevm]

What is this feature about? (1 sentence)

Discuss project structure changes which make it more straightforward where to put new UI/Logic

Why is it needed? What is the value? For whom do we build it?

Currently, project structure is a little bit messy. We have routes folder which contains a mix of UI/LOGIC and logic folder which contains only logic. It's not clear where to put new things.

High-level overview of the feature

My suggestion: split ui/logic, move logic outside of routes to logic, so routes will only contain UI code. But this is open for a discussion.

[nicosampler]

I would suggest something like this:

inside src

• /logic
• /utils
• /types
• /config
• /theme
• /components
• /containers
  
  
  
  • /landing
  
  
  • /add-safe (creation and load)
  
  
  • /safe
  
  
  • /balances
  
  
  • /transactions
  
  
  • /apps
  
  
  • /address-book
  
  
  • /settings
  
  

[mikheevm]

@nicosampler not a big fan of it, I like the idea of splitting components by their usage on a particular route

Also, this doesn't really address the problem described in ticket (logic being defined in both routes/logic folders)

[nicosampler]

@mikheevm it says:

We should discuss and change the project structure

I'm not a big fan of the current structure, I always get lost in there and I need to find the component I need to work on by inspecting the dom structure using the devTools.

But perhaps the rest of the team feels comfortable with the current structure.

[mikheevm]

@nicosampler sorry about not making it a little bit more concrete, this ticket is an evolution of the discussion started there: https://github.com/gnosis/safe-react/pull/1089#pullrequestreview-453358789

I'm not a big fan of the current structure, I always get lost in there and I need to find the component I need to work on by inspecting the dom structure using the devTools.

How is this related to project structure? I can find myself in a similar position when starting to work on a new unfamiliar project, but honestly I don't see how changing project structure could fix that

[pablofullana]

@mikheevm @nicosampler are there any industry-wide best practices or well-known project structures that are proven to be ok and have been adopted as standards? I guess it would be a good practice to compare current project structure against any standard out there.

[nicosampler]

Agreed with Pablo, let's try to peg to a standard.

Following a routes approach, in my opinion, is not a good practice, I prefer to follow what app show when you open it. So each "container" represents a state/functionality in the app, otherwise, a developer should know all the possible routes in order to find a component in the project structure.

By looking at the structure I proposed, it's pretty simple to find/know where the code of each part of the app is located.

Perhaps @fernandomg and @dasanra could give their opinions.

[mikheevm]

@nicosampler @pablofullana theres no such standard, also, we don't want to overthink it in a way that we have to restructure the whole project, splitting logic and ui is far more easy

[mikheevm]

@nicosampler

Following a routes approach, in my opinion, is not a good practice, I prefer to follow what app show when you open it.

This doesn't make any sense, why is it not a good practice? How is your structure going to help with understanding what app shows? How does one understand that he now sees a particular container/component and how does it correlate with project structure?

IMO routes structure is simple and easy to follow:
route/index.js - describes the layout for the whole route page
route/*.js - components/containers/etc unique to this route

Let's also not forget that there's no ideal approach and every approach has its tradeoffs, but I don't understand your concern, IMO it works just fine for us

[mikheevm]

https://reactjs.org/docs/faq-structure.html

One common way to structure projects is to locate CSS, JS, and tests together inside folders grouped by feature or route.

[katspaugh]

Closing this issue, will restart this discussion in a different format.