Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SIP-61] Improve the organization of front-end folders #13632

Closed
michael-s-molina opened this issue Mar 15, 2021 · 26 comments
Closed

[SIP-61] Improve the organization of front-end folders #13632

michael-s-molina opened this issue Mar 15, 2021 · 26 comments
Labels
sip Superset Improvement Proposal

Comments

@michael-s-molina
Copy link
Member

michael-s-molina commented Mar 15, 2021

[SIP-61] Proposal for improving the organization of front-end folders

Motivation

The major motivation of this PR is to propose a better organization for our front-end folders.

This is our current structure:

branding (old files)
cypress-base (e2e tests)
images (images)
spec (unit and integration tests)
stylesheets (less files)
src
   addSlice (page)
   api (one service)
   chart (components)
   common (common components and hooks)
   components (common components)
   CRUD (components)
   dashboard (page)
   dataMask (action, reducer and type)
   datasource (components)
   explore (page)
   filters (components)
   logger (utilities)
   messageToasts (components)
   middleware (initialization)
   modules (utilities)
   profile (page)
   setup (initialization)
   showSavedQuery (utilities)
   SqlLab (page)
   staticPages (assets)
   types (generic types)
   utils (utilities)
   views (multiple pages)
      CRUD
         alert (page)
         annotation (page)
         annotationlayers (page)
         chart (page)
         csstemplates (page)
         dashboard (page)
         data 
            database (page)
            dataset (page)
            query (page)
            saved query (page)
         welcome (page)
   visualizations (components)

As you can see I tagged each folder with its respective role in the application. This structure has some problems:

  • Lack of organizational consistency. We don’t know if the folders are organized by features, pages, technical layers, modules, etc. They are all mixed.
  • Lack of naming consistency. Some folder uses camel case pattern while others use snake case (SqlLab, addSlice, annotationlayers).
  • Lack of test coverage. We have separate folders for tests and components (src e spec). The spec folder tries to mimic the same structure of the src folder but has a lot of folders and components missing. This indirect relationship contributes to decreasing test coverage.
  • Lack of documentation about the structure and its justifications. New contributors don't know where to develop new features or have difficulty finding a particular feature.

With that in mind, this PR tries to achieve the following:

  • Create an organizational structure based on common best practices but tailored to our project
  • Create a document about the organizational structure for the community
  • Remove conceptual conflicts
  • Fix folder naming inconsistencies
  • Create an organizational structure that encourages testing and reduces coverage problems
  • Guide new features

Proposed Change

The proposed change is based on a number of resources available. Here are some of them:

I took the liberty to paste some quotes from the sources:

Organizing your code by modules means you start thinking around a concept where you break down your code into related pieces and provide a public interface to use your module. It helps maximize code sharing and reusability in different sections of your application and even on other projects.

Another important attribute of a module-based structure is the ability to replace one by another, as long as both modules meet the same requirements.

In a world where everything moves fast and new features replace old ones in no time, breaking your code by modules will give you the freedom to build multiple versions of them to facilitate A/B testing and fully replace old ones when your team is satisfied with the result.

Each component, scene, or service (a feature) has everything it needs to work on its own, such as its own styles, images, translations, set of actions as well as a unit or integration tests. You can see a feature like an independent piece of code you will use in your app (a bit like node modules).

Code is structured in a way that reflects purpose.

Code is easier to refactor.

Organizing an application as a series of features can make feature flags easier to implement.

The idea is to better organize our code structure to reflect a more modular approach. Applying those concepts to our code base is an iterative process by nature, so the following proposed structure is the result of the first iteration:

cypress-base (e2e tests)
src
   assets (common assets)
      images (images)
      staticPages (assets)
      stylesheets (less files)		
   components (common components)
      chart (components)
      dataMask (action, reducer and type)
      datasource (components)
         CRUD (components) - this folder won’t exist. It’s here just to show where the components went.
      messageToasts (components)
      visualizations (components)
         filters (components)
   hooks (common hooks)
   pages
      addSlice (page)
      alert (page)
      annotation (page)
      annotationLayers (page)
      chart (page)
      cssTemplates (page)
      dashboard (page)
      database (page)
      dataset (page)
      explore (page)
      profile (page)
      query (page)
      savedQuery (page)
      sqlLab (page)
         api (one service)
      welcome (page)
   setup (initialization)
      middleware (initialization)
   types (common types)
   utils (utilities)
      logger (utilities)
      modules (utilities)
      showSavedQuery (utilities)

These are the changes:

  • All folders with camel case naming.
  • The majority of articles put the assets inside src, so we can move images, stylesheets, and staticPages to a folder named asset under src.
  • Only one image of the branding folder is referenced and the same image is present in the images folder. So we can remove the branding folder and update the reference.
  • All components that are shared across the application are inside the components folder. A component that is specific for a page should go inside that page.
  • All common hooks are inside the hooks folder.
  • The pages folder is the heart of the application. All the various features/screens/pages are defined here. Each page will have a components dir. This will hold all the components that are required by only this page.
  • The setup folder is responsible for the initialization code.
  • The types folder are common types used throughout the application. When possible we should define types in the same files of their components.
  • The utils folder contains all the utilities.
  • All the specs were moved to the src folder alongside each component/page/hook/etc. The same is valid for mocks. When restructuring we should verify if a mock is shared or belong to a specific module.
  • I opted for leaving e2e tests outside of src because it seems to be the common practice and because these tests are designed to span across multiple pages and workflows.

Looking at a more detailed view of the structure, we should strive to organize our code in a more modular way. Some examples here for demonstration purposes:

An example of a common component with storybook and tests:

src/components/Button
   Button.test.tsx
   Button.stories.tsx
   index.tsx

An example of a page:

src/pages/sqlLab
   actions
      sqlLab.ts
      sqlLabl_spec.tsx
   components
      QueryTable
         index.tsx
         QueryTable.test.tsx 
      ResultSet
         index.tsx
         ResultSet_spec.tsx
   reducers
      sqlLab.ts
      sqlLab.test.tsx
   utils 

Here we can see that we can create nested folders when necessary to better organize our code. Also, the most important aspect here is that the tests are close to the component and this will help to spot coverage problems in our application.

In the future we can iterate more on this structure until we reach something like this:

src/pages/sqlLab
   FeatureA
      actions
      components
      reducers
   FeatureB
      actions
      components
      reducers

When we reach this point, all related code will be grouped together and it will be even easier to work with feature flags and A/B feature substitutions.

The proposed structure is not final. It is the result of the first iteration following the organizational principles described here. As we move forward in the organization of the folders, we can make different decisions motivated by technical restrictions or a better understanding of a component. The idea is to start the code organization by taking an iterative approach and construct a guideline in which all organizational decisions are justified and can be easily understood by the community.

New or Changed Public Interfaces

The intent of this PR is to not change any public interface.

New dependencies

No new dependencies.

Migration Plan and Compatibility

The idea here is to promote this organization through small PRs, using an iterative approach, to avoid abrupt changes and reduce conflicts with open PRs. Similar to what we did in the components folder. After every step, we should assess and review the structure.

We should also create from the start the documentation about the structure. We can write a section in CONTRIBUTING.MD or, because of CONTRIBUTING.MD size, another file like FRONTEND_STRUCTURE.MD. This documentation will guide the efforts and serve as a reference for the community.

Rejected Alternatives

Another popular way to structure projects is to group similar files together, by file type or technical layers. This could be applied to small to medium projects but doesn’t scale as well as grouping by features.

Keep the structure as it is was also rejected because of all the problems listed above.

@junlincc junlincc changed the title [SIP] Improve the organization of front-end folders [SIP-61] Improve the organization of front-end folders Mar 15, 2021
@junlincc junlincc added the sip Superset Improvement Proposal label Mar 15, 2021
@junlincc
Copy link
Member

cc @kristw @ktmud

@ktmud
Copy link
Member

ktmud commented Mar 15, 2021

Thanks for the thoughtful proposal and comprehensive reasoning, @michael-s-molina ! I think this totally makes sense. Let's do it!

@kristw
Copy link
Contributor

kristw commented Mar 15, 2021

sgtm!

@suddjian
Copy link
Member

suddjian commented Mar 15, 2021

This looks great! Thank you for the well organized and researched SIP.

I suggest removing "utils" as a category. In my experience, utils usually ends up being a disorganized grab bag of random things. I'd move "logger" to a "services" category, and split out the rest into either their feature folders, or into files organized by their domain (stringFns.ts, urlFns, etc.).

@srinify
Copy link
Contributor

srinify commented Mar 15, 2021

Love this!

@eschutho
Copy link
Member

Awesome @michael-s-molina! What do you think about putting the redux actions and reducers outside of the folders for the pages or features as they will usually align with the structure of the store and not necessarily be that closely related to the rest of the folder structure?

@michael-s-molina
Copy link
Member Author

Thank you all for your feedback! I really like the spirit of this community!

I suggest removing "utils" as a category. In my experience, utils usually ends up being a disorganized grab bag of random things. I'd move "logger" to a "services" category, and split out the rest into either their feature folders, or into files organized by their domain (stringFns.ts, urlFns, etc.).

Awesome @michael-s-molina! What do you think about putting the redux actions and reducers outside of the folders for the pages or features as they will usually align with the structure of the store and not necessarily be that closely related to the rest of the folder structure?

@suddjian and @eschutho Your comments are great and couldn’t be more related! I was reviewing the articles and I think we can kill two birds with the same stone!

Some quotes from React Folder Structure in 5 Steps:

From here, there may be other utilities which need to be accessible to your components and hooks. For miscellaneous utilities I usually create a services/ folder.

Fortunately JavaScript's Intl API gives us excellent tools for date conversions. However, instead of using the API directly in my React components, I like to have a service for it, because only this way I can guarantee that my components have only a slim set of actively used date formatting options available for my application. Otherwise there would be lots of different date formats in a growing application.

Furthermore, if a service from the previous step is tightly coupled to a domain, then move the service to the specific domain folder.

This is a picture from the article showing the organization:

Screen Shot 2021-03-16 at 2 38 34 PM

We can see that the organization follows the same logic as the components but this time using a services folder. So if a service is shared among features then it belongs to the first level. If it's something specific then it should go to the feature level. We can apply the same concept to utilities, actions and reducers. We can see in the image that date formatting (utility) is treated as service and that any level can contain a service folder depending on reusability.

The same pattern is present in How to better organize your React applications?

Screen Shot 2021-03-16 at 2 43 43 PM

Sounds good?

@suddjian
Copy link
Member

Sounds great!

@eschutho
Copy link
Member

I feel that "store" and "hooks" are two sides of the same coin. If we're going to put hooks in a folder, it would make sense to me to put all of the store reducers and actions in a folder together as well, beside the hooks. Depending on how we structure the store after the single page app transition, I'm not sure that each file would line up with a "service". It's also helpful to be able to see all the reducers and actions in one place to get a sense of the structure of the store, imo. Thoughts on that?

@eschutho
Copy link
Member

eschutho commented Mar 17, 2021

There will be a few setup storybook files that need to go somewhere... would those go in setup? One other small nit, can we also move toward getting all the folder/file names to be nouns? (I think addSlice is the only one for now)

@michael-s-molina
Copy link
Member Author

@eschutho Thank you for your comments.

It's also helpful to be able to see all the reducers and actions in one place to get a sense of the structure of the store, imo. Thoughts on that?

Redux official documentation has a very nice guide on best practices for code structuring and they also recommend using feature folders instead of having all the reducers and actions in one place:

We specifically recommend organizing your logic into "feature folders", with all the Redux logic for a given feature in a single "slice/ducks" file".

I also think it’s helpful to view the structure of the store. I just think that the best way to view that is using tools that assemble the store structure, especially when we use feature flags that dynamically alter this structure. Think about feature substitution, I can replace a feature and all reducers and actions that are specific to that feature can be replaced. Maybe the new feature doesn’t even use redux. So the structure of the store really depends on which features are enabled at the moment.

I feel that "store" and "hooks" are two sides of the same coin. If we're going to put hooks in a folder, it would make sense to me to put all of the store reducers and actions in a folder together as well, alongside hooks.

I understand your point. Maybe we can treat them as services as well. Our current hooks include date formatting, string manipulation, URL handling, etc.

Depending on how we structure the store after the single page app transition, I'm not sure that each file would line up with a "service".

In this type of scenario where we still don't have all the information available, I tend to tackle the problem in steps and delay the decision to a better point in time. So I think we can aim to keep all organized for now and reevaluate after we finish the single page app transition.

Simplifying even more we could have:

src
   assets
   components
   pages
      sqlLab
        actions (feature actions)
        hooks (feature hooks)
        reducers (feature reducers)
   services
      authentication 
        actions (service actions)
        hooks (service hooks)
        reducers (service reducers)
      dateFormatting (utility)
      setup (initialization) -- Moved to services
      stringManipulation (utility)
      urlHandling (hook)
   types

Since this is an iterative approach I think this is a good starting point. When we have all code organized, and all the baggage learned while refactoring, it will be easier to evaluate if we need to further break the services folder into more categories.

There will be a few setup storybook files that need to go somewhere... would those go in setup?

I'm assuming you're talking about the plugins storybook right? If that's the case I think that plugins should have the same structure as the components folder where the storybook and tests are alongside the component code. SIP 58 already described this possibility:

We can create a main storybook folder and write each story in it's own plugin. The main storybook will collect all stories from every plugin/package.

If you are talking about .storybook folder then it can just stay where it is.

One other small nit, can we also move toward getting all the folder/file names to be nouns? (I think addSlice is the only one for now)

Totally agree.

@ktmud
Copy link
Member

ktmud commented Mar 18, 2021

I think it's OK to have an utils folder, as long as functions within utils are categorized and put under one category per file and properly named. Calling things like stringManipulation and dateFormat services is kind of weird.

@michael-s-molina
Copy link
Member Author

michael-s-molina commented Mar 18, 2021

@ktmud Indeed the utils folder was present in the majority of the resources that I used for research. I also do understand @suddjian worries. Maybe we could reach a middle ground by revising our current utils folder, making sure that all files adhere to the requirements that you mentioned, and applying @suddjian's suggestion of moving all non-compliant to their respective features. @suddjian what do you think?

@eschutho
Copy link
Member

Since this is an iterative approach I think this is a good starting point. When we have all code organized, and all the baggage learned while refactoring, it will be easier to evaluate if we need to further break the services folder into more categories.

Thanks @michael-s-molina. In my experience with redux, the services don't always (or usually) line up with the services, but they somewhat do for us for the most part, so I'm good with us trying this out for now. I think we should be mindful not to let the folder structure influence our store design. The best store designs that I have seen have little to do with the code or feature structure, so just as long as we keep an open mind as this evolves. :)

@michael-s-molina
Copy link
Member Author

I think we should be mindful not to let the folder structure influence our store design. The best store designs that I have seen have little to do with the code or feature structure, so just as long as we keep an open mind as this evolves. :)

Totally agree with you @eschutho! The open mind principle will be applied to all the decisions made here. I really think that once we have the first wave of organization, more will come and that can include reviewing some decisions! 👊🏼

@rusackas
Copy link
Member

Thank you @michael-s-molina - it seems like the discussion has settled down here quite a bit, and people seem at ease with the general intent and organizational concepts of this plan. If we run into any situations that raise further concerns in how we organize certain pieces (e.g. redux store structure, or utils becomes a junk drawer), we can bring the discussion back here for further elaboration and alignment. I know that I, for one, see more value to this effort than risk, so with that, I'm happy to put it up for a vote.

@rusackas
Copy link
Member

rusackas commented Apr 6, 2021

The vote has officially passed! 🎉 Thanks @michael-s-molina for all the fantastic work here!

@rusackas rusackas closed this as completed Apr 6, 2021
@simcha90
Copy link
Contributor

I have a question here, if I have some test utils relevant for only for some specific folder like integration tests for FiltersBar folder, where I can put these utils?

@michael-s-molina
Copy link
Member Author

I have a question here, if I have some test utils relevant for only for some specific folder like integration tests for FiltersBar folder, where I can put these utils?

@simcha90 If it's specific for FiltersBar it should go inside the FiltersBar folder. You can opt to leave it at the root level or if you have more utils, create a utils folder inside FiltersBar folder.

FilterSets has an example of this structure:

Screen Shot 2021-04-13 at 9 34 23 AM

@rusackas
Copy link
Member

rusackas commented Feb 7, 2024

I plan to add linting rules here to track the remaining technical debt here and/or reinforce these standards. Just FYI.

@rusackas
Copy link
Member

Anyone know what the next step(s) here ought to be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sip Superset Improvement Proposal
Projects
Development

No branches or pull requests

9 participants