veritus / veritus-web

Frontend
https://deploy-preview-32--announcer-tachometer-25258.netlify.com/
2 stars 0 forks source link

File structure #40

Closed AriHrannar closed 7 years ago

AriHrannar commented 7 years ago

The file structure right now is pretty much just

src ---- Components ---- Containers

This will definitively get clustered soon as we add more containers and components. Then when we start adding redux, creating utils functions and tests it will get very clustered

What about moving to something like this: https://marmelab.com/blog/2015/12/17/react-directory-structure.html

Where the structure would be

src ....promises ......... components ......... containers ......... actions ......... reducers ......... utils ....cases ......... components ......... containers ......... actions ......... reducers ......... utils

Ragnar-H commented 7 years ago

The file structure is actually

/src/Components/
..../LoginForm/
......../__tests__/
............index.test.js
......../index.js
.
.
.
/src/Containers/
..../LoginContainer/
........index.js
.
.
.

Which, if Redux is added, would be expanded to

/src/Components/
..../LoginForm/
......../__tests__/
............index.test.js
......../index.js
.
.
.
/src/Containers/
..../LoginContainer/
......./__tests__/
...........reducer.test.js
...........actions.test.js
........index.js
........reducer.js
........actions.js
.
.
.

The beauty of this file structure, is that all Redux logic goes into containers, so if you want to change to Flux, the next big thing, it's all contained within here. Containing the Redux logic in the Containers (hehe) allows us to write purer React components in the Components directory. Which I've got quite a vested interest in trying.

This structure is similar to the Ducks file structure.

I understand what he is doing in A Better Structure for React/Redux but it is too tightly coupled to Redux IMO.

The biggest proponent for why I'm advocating this file structure is because it fits incredibly well into my workflow. Creating PoliticianCard f.x.:

  1. Create a React component that returns a div with the text "Im a PoliticianCard"
  2. Create a Storybook for PoliticianCard
  3. Develop the PoliticianCard in isolation in Storybook, allowing me to get the API(props) just right
  4. Create PoliticianContainer which makes HTTP calls to the backend

This isolated development really allows me to focus on getting one thing right at a time.

Ragnar-H commented 7 years ago

And if you start creating Component and Containers in the same file, with a default export, I'm going to 😭

AriHrannar commented 7 years ago

You dont think it will create any issues down the road that the containers and components are not close to each other?

But yeah we can keep the current structure and just go with what you said if its easier that way

Ragnar-H commented 7 years ago

Well there are always tradeoffs.

And it shouldn't just be what I say is easy. These are my arguments for the type-of Ducks file structure, I'd love to hear more arguments for your proposed method. I'm not convinced by the article (and neither are most of the comments it seems) and I simply cannot wrap my head around having Containers and Components in the same file

AriHrannar commented 7 years ago

We dont have them in the same file at work at least, does the article suggest that? Its been a few months since I read it.

Our structure is pretty much this:

src
....promises
......... components
................. Promise
................. Promise.spec.js
......... containers
................. PromiseContainer
................. PromiseContainer.spec.js
......... actions
................. promiseActions.js
................. promiseActions.spec.js
......... reducers
................. promiseReducers.js
................. promiseReducers.spec.js
......... utils
................. promiseUtils.js
................. promiseUtils.spec.js
Ragnar-H commented 7 years ago

Looks much saner when you put files into it. Where do the tests for actions and reducers go? And could you provide an example of what goes into these domain specific utils folders?

On Tue, 13 Jun 2017, 22:22 Ari Hrannar Bjornsson, notifications@github.com wrote:

We dont have them in the same file at work at least, does the article suggest that? Its been a few months since I read it.

Our structure is pretty much this:

src ....promises ......... components ................. Promise ................. Promise.spec.js ......... containers ................. PromiseContainer ................. PromiseContainer.spec.js ......... actions ................. promiseActions.js ......... reducers ................. promiseReducers.js ......... utils ................. promiseUtils

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/veritus/veritus-web/issues/40#issuecomment-308237018, or mute the thread https://github.com/notifications/unsubscribe-auth/AVy8QugKJ7awU7igrHzQ5sgJ43Hv2JD0ks5sDu-OgaJpZM4N4pIL .

AriHrannar commented 7 years ago

Updated the comment above to include tests

Utils examples that I can think of right now:

  1. We use JSON Web Tokens and they need to be parsed to get all kinds of data. We have a function in authenticationUtils that parses the token and returns the data within it (f.x. expiration date, user email, user id, other user data)
  2. Functions that calculate from one measurement system to another (f.x. kilograms to stones/pounds and back again, some other health stuff we calculate)
  3. Validation functions (password, SSN, phone nr, etc)
Ragnar-H commented 7 years ago

Makes sense.

My only quarrel with the structure is the placement of tests. I'm a fan of placing tests in __tests__ directories next to whatever they're testing.

That way they're not cluttering the file view unless I want to touch them.

spec vs test is another thing entirely 😅 Create React App will run either of them happily, but I've yet to write a specification in JS