Brianzchen / scaled-fractal

Fractal based design pattern for large scaled applications with many teams
MIT License
2 stars 3 forks source link

Questions and suggestions #5

Open josephj opened 6 years ago

josephj commented 6 years ago

Hey Brian,

Thanks for sharing this idea. It’s really nice to think about the large-scale front-end architecture. I rarely have the opportunities to do that in startup environment. Some ideas are really awesome! I love how you make it possible for other view libraries work. Besides it still keeps the flexibility for everyone else to jump in.

I think the most important of thing is about your dev teams’ R&R and workflow. I am just an outsider so my suggestions or questions might not be very relevant to you. Just take as reference and keep having discussions, you will find the best practice for your company.

Suggestion - Merging modules & packages

I can’t see the benefits of separating files to 2 folders. All the files are highly related with each other. While we use the term module, it usually means self-contained and can be reusable as well. Thus I would suggest to combine these 2 folders together.

    [modules/Menu]
    ├── actions.js
    ├── components
    │   └── Container.js
    ├── constants.js
    ├── index.js
    ├── initialState.js
    ├── package.json
    └── reducer.js

Details

Benefits

Suggestion - Redux Store Convention

It might be good to have a clear namespace convention so that developer in different team doesn’t have to guess. For example, using the path as the store key.

    {
      "modules": {
        "Menu": {
          "open": false
        },  
        "Header": {...},
        "Main": {...}
      },
      "router": {...}
    }

The action constants should follow this as well.

    // modules/Menu/constants.js
    const prefix = 'modules/Menu/';
    export const OPEN_MENU = `${prefix}OPEN_MENU`;

You can make use of the CI build to ensure all module developers follow this namespace convention.

Question - How to Override styles?

It’s a common that a component inside different parents has minor style difference. Having an easy way for a parent component to override its children style is required. Not sure if aphrodite provides a straightforward way to achieve this.

    import Header from 'modules/Header';
    import styled from 'styled-components';

    const App = () => (
      <div>
        <StyledHeader/>     
      </div>
    );

    const StyledHeader = styled(Header)`
      a:link, a:visited {
        color: blue; // overriding module style according to parent need.
      }
    `;

Question - Handling the API Data

The only relevant mention is the src/services folder. I am curious how will you make the data being used efficiently across all the different modules.

You might be interested how we solve this problem.
https://github.com/efcsydney/efcsydney-roster/tree/master/client/src/resource

    // ModuleA.js
    import { createApiActions } from 'resources';

    const {
      createEvent,
      createEventComplete,
      createEventReset,
      modifyEvent, 
      modifyEventComplete,
      modifyEventReset,
      destroyEvent,
      destroyEventComplete,
      destroyEventReset,
      retrieveEvent,
      retrieveEventComplete,
      retrieveEventReset
    } = createApiActions('events');

    withResource('events')(
      class EventList extends Component {
        handleClick = data => {
          this.props.dispatch(modifyEvent(data));
        }
        render() {
          const { events } = this.props;
        }
      }
    );

Suggestion - Using redux-actions (Minor)

Less boilerplate code for actions, constants and reducers.

    // redux.js
    import { createAction, handleActions } from 'redux-actions';

    export const fetchEvents = createAction(`${prefix}RETRIEVE_EVENTS`);
    export const fetchEventsComplete = createAction(`${prefix}RETRIEVE_EVENTS_COMPLETE`);
    export default handleActions({
      [fetchEvents]: () => return {...};
    }, initialState);
Brianzchen commented 6 years ago

Hey @josephj,

Awesome suggestions, and really cool stuff to think about. Thanks so much for taking the time to review my work too.

I'll try to address them one at a time and see how we go from there

Merging modules & packages / Redux Store Convention


I think the first two suggestions come hand in hand so I'll try and discuss them together.

The main reason for the separation of modules and packages was simply to improve overall readability in the project. My vision was that modules held react components and simply that (though I mean this in the loose sense like how I demonstrated we should be free to try new view libraries). If I were to look in the modules folder, I would know that everything here is what the user sees as opposed to something functional. I don't feel it's quite right to dump everything into one root and then as you scale you loose track of everything. Perhaps my usage of the term module was a bit too all encompassing? Perhaps something like views?

I'll start using the term view from now, hoping that's more clear.

In terms of the redux store convention, this is more of a question back to you for your thoughts. But for me, the store shouldn't be a hard representation of what the ui looks like, it's more like a client-side database of what the user can currently see. The best example of this case would be a user state, is the user logged in or not, perhaps they are gated by some kind of premium membership and should be able to access particular content. information like this is kept in the overarching store, but accessible by any component that needs this information. Another good example is, how we always want to keep the redux store normalised. We may have many objects containing lists of data, while one other list that holds the common id to track across the application lifecycle There would of course be cases where a particular state property that's kept in redux is only used by one entity, in that instance I would assume that the maintainers of this view would also build a matching slice of reducer to store their state.

General opinion is that, business state and views are a decoupled entity, they aren't 1 to 1, they might come close but never be exact.

It seems like this is the common approach, looking at how sites like reddit implement their store using redux-tool extension. But open to new ideas and thought processes

What do you think of this concept?

In relation to your suggestion about keeping more of my module together, although I've given my opinions of the view side, I can now see how my constants/actions/reducers are a bit too far apart, and I believe I should make some changes to adjust this and put the reducers closer to their respective actions/constants.

How to Override styles?


Aphrodite supports concat of styles with this syntax, className={css(style1, style2, props.classes)}

As I mention in my readme, one view rendering another, shouldn't have any props passed through as this makes the view not consistent across the application if reused.

In terms of reusable atomic components, I think that really depends on the component library the project chooses to use, though if I were to implement the reusable library as well, my vision would be that these components would accept a prop called classes and then components would interpret that and add them as an override however that may happen, which depends on the library itself.

Handling the API Data


Yes! I agree I should probably add more context on this, I do have a vision of how this should function but I guess I was simply lazy.

But the idea is that, in the src/index.js, I have injected the service into thunk's withExtraArgument param, which would make the service only accessible from redux thunk actions. This creates a level of consistency where you wouldn't want service calls happening from just anywhere and you have it under control in a way.

Using redux-actions (Minor)


I can see the lesser boiler plate, though this just a personal preference on my end, I like the verbosity when writing out my own actions and reducers. It's not a large overhead, and it also keeps things understandable. Pretty much, if I have a clear understanding of how JavaScript works, I'll be very confident with handling everything redux also.


If you want to respond back to anything that'll be really cool! And this was honestly super helpful for me, although I don't currently agree with everything, it's given me so much more to think about and I definitely want to make some changes

josephj commented 6 years ago

Hey man,

Really glad you feel my response is a bit useful. I love to have constructive conversations. 🤘

Merging modules & packages

Not sure if I understand correctly. Maybe you feel organising by object role is easier for scaling, rather than organising by feature. There are some different thoughts.

I don't mind to join either team using different folder/file structure. However, I feel your objective is for multiple teams. What's the easiest way for others to join? The user journey with existing structure might a bit difficult from my imagination.

  1. Create component under views folder, copy all my team style setting and create package.json.
  2. Create action under packages folder, copy all my team style setting and create package.json.
  3. Write related reducer undersrc/reducers folder and mount it, but I can't use my team's code style.
  4. Have to start the whole app for development.
  5. During the development, I have to keep jumping between different folders or searching for my files.

Though it helps for the readability you mentioned but it might be more difficult while developing and less testable. If I were the architecturer, I want to choose which way is more beneficial to others.

Redux Store

Sorry for misunderstanding. I suggested that because currently you only have the UI state (menu.open) in the example. I don't mind it's pure presentation while the context is the UI state. I just want consistency and ease to discover these UI state. It's possible that other teams mount their UI state or name their actions casually. It's better to have a store structure in the beginning.

// store snapshot
{
  entities: {
    user: {
      logged: true,
      tier: 'premium'
    },
    events: {
      '5ac560706eab2dc4e8539df4': {...}
    }
  },
  views: {
    menu: {
      open: false
    }
  }
}

// constants
const OPEN_MENU = 'views/menu/open'; // namespaced with views

// action creator
function openMenu() {
  return {
    type: OPEN_MENU ,
    payload: true
  }
}

Agreed that the business state and data state should be decoupled. And definitely the data state should be normalised. No problem at all. 👍

Override Styles

All good 👍

Handling the API Data

Yes, that will be really nice to see. It might be the most important thing to demonstrate the data flow and how it's being stored in redux.

Using redux-actions

No problem at all. Agreed it's personal preference! 😊


The above might just my personal feeling. Maybe not a problem for anyone else. It's more important to ask how your co-works feel. They are your users. My opinion can be ignored anyway 😂