gitpoint / git-point

GitHub in your pocket :iphone:
https://gitpoint.co/
MIT License
4.72k stars 788 forks source link

Modify directory structure #506

Open housseindjirdeh opened 7 years ago

housseindjirdeh commented 7 years ago

Our folder structure has changed as things have progressed and I'm thinking of another pattern we can start moving towards. I think having a folder structure for every component might be something we need to move towards. For example, our low level components in components/

- components/
  - Button/
    - button.component.js
    - button.styled.js
  // ...

And our container components (our "screens")

- screens/
  - notifications/
    - notifications.component.js
    - notifications.styled.js
    - notifications.container.js
    - notifications.action.js
    - notifications.reducer.js
    - notifications.selector.js
    - notifications.route.js
    - notifications.type.js
 - organization
    // ...

With this pattern each of our components will be purely JSX renders that accept props. A container file like notifications.container.js will be something like the following:

import { compose } from 'recompose';
import { connect } from 'react-redux';
import { mapDispatchers } from 'utils';

import { Notification } from './notification.component';
import { notificationConnector } from './notification.selectors';

import { getUnreadNotifications } from './notification.actions';
import { getPendingNotifications } from './notification.actions';

const dispatchers = mapDispatchers({
  getUnreadNotifications,
  getPendingNotifications,
});

export const NotificationContainer = compose(
  connect(notificationConnector, dispatchers),
)(Notification);

Keeping this logic separate from the actual component file means the actual notification component file does not represent any specific state logic. It just takes props, and the container is responsible for mapping those props to states and actions. In this example, also taking advantage of recompose. mapDispatchers can also be a utils method that leverages bindActionCreators from redux.

This is a pattern a lot of my colleagues here at work are using and I'm really starting to see the benefit. Will love to hear everyone's opinions šŸ’¬

andrewda commented 7 years ago

I like it! As just a minor phrasing thing, I think it might be better to use button.style.js instead of button.styled.js (obviously very minor and unimportant).

housseindjirdeh commented 7 years ago

^ Nope that's definitely important, and I agree :)

machour commented 7 years ago

I like this a lot!

Just to avoid confusions as the notifications section currently only contain one screen, how would src/user and the 4 screens it contain be adapted? Would it be one root folder per screen?

screens/
ā”œā”€ā”€ user-followers-list/
ā”œā”€ā”€ user-following-list/
ā”œā”€ā”€ user-profile/
ā””ā”€ā”€ user-repositories-list/
housseindjirdeh commented 7 years ago

<3

Yep @machour that's exactly what I was thinking . Right now we have a number of screens within user currently and they could all be in their own separate directories like this:

components/
ā”œā”€ā”€ button/
ā”œā”€ā”€ badge/
ā””ā”€ā”€ // ...
screens/
ā”œā”€ā”€ notifications/
ā”œā”€ā”€ user-followers-list/
ā”œā”€ā”€ user-following-list/
ā”œā”€ā”€ user-profile/
ā”œā”€ā”€ user-repositories-list/
ā””ā”€ā”€ // ...

With this structure, it will mean we'll have to split user.action.js to 4 different unique action files (and the same with user.reducer and user.type). It's going to be more work but I think the granularity will only make things better.

If a screen means it's a container that maps to our state, then automatically make it have it's own root folder. If it's a low level component that doesn't map to state, we throw it in the components directory.