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

Frontend Modularization Planning #398

Closed
AmrSaber opened this issue Mar 31, 2023 · 9 comments
Closed

Frontend Modularization Planning #398

AmrSaber opened this issue Mar 31, 2023 · 9 comments
Labels
priority: high status: in-progress This issue has been picked and is being implemented type: modularization Code refactoring project

Comments

@AmrSaber
Copy link
Contributor

AmrSaber commented Mar 31, 2023

Frontend Modularization Plan

Current Frontend Structure

The current structure of the frontend application is as follows

src
├── api -- contains integration API files
├── components -- All components used in the project
├── contexts -- Used contexts (provide some info to app)
├── hooks -- All hooks used in the project
├── icons -- All icons used in the project
├── store -- Common redux store files
├── theme -- Themes used in the app
├── utils -- Utilities used in different parts of the application
├── views -- All the screens in the application
├── App.js -- React root
├── index.js -- Project root
├── routes.js -- Project routes
└── constants.js -- All Constants used in the application

As you see, the files are structure based on their logical properties, and not based on their module. But a lot of the directories already contains common code used by different parts of the application that we could consider modules, we will make use of that fact later on.

The Problem With The Current Structure

The project is currently structured in a manner that describes the "What" of each component of the code. As the scope and the size of the project get bigger and bigger, this structure gets in the way of contributing to the project because of 2 problems:

  1. To update an existing module, you will need to go through all the directories and figure out which of them is related to the module you want to update
  2. To add a new module, the process is not well defined at all, you will need to go around the project creating files everywhere which also increases the difficulty of the first problem

Because of this, we agreed that we will split the project into meaningful modules, so that contributing to an existing module is almost trivial, you will know exactly what files are related to that module. And also adding a new module will be a well defined process so that the project is kept organized.

Modular Structure

src
├── authentication
│   ├── components
│   ├── contexts
│   │   ├── AuthContext.js
│   │   ├── CognitoAuthContext.js // Previously AmplitudeContext.js
│   │   ├── LocalAuthContext.js // Previously LocalContext.js
│   │   └── index.js
│   ├── hooks
│   └── index.js
├── design
│   ├── components
│   ├── contexts
│   │   ├── UXSettingsContext.js
│   │   └── index.js
│   ├── hooks
│   ├── icons
│   ├── themes
│   └── index.js
├── globalErrors // Previously store
├── modules
│   ├── Misc
│   ├── MLStudio
│   ├── ...
│   └── Notebooks
│       ├── components
│       ├── hooks
│       ├── services
│       ├── views
│       └── constants.js
├── services // Previously api
│   ├── graphql
│   └── hooks
├── utils
├── App.js
├── index.js
└── routes.js

The description of each of the new directory is as follows:

  • modules: The modules that we agree upon (more about modules in the following notes section)
  • authentication: contains (components, hooks, and contexts) related to user authentication
  • design: contains (components, contexts, hooks) that are used for or directly affects the UI
  • global errors: previously called "store" contains the global error handling for the project as the redux store was only used for error handling
  • services: previously called "api" contains the graphql schemas and hooks necessary to integrate with BE
  • utils: common helpers that are used across the app

This structure organizes the files depending on the "Why" that's why common functionality are not just grouped into common but they are moved into a directory that describes the functionality they provide, such as (authentication, design, errors, and services).

Notes

  • a view represent a screen that is in the application, while a component is a section of a screen, this can be specific to that screen and is used for abstraction or simplification or it could be a common component that is used to in different screens
  • About modules
    • Each module is based around its views (screens)
    • Each of (components, hooks, and services) directories are for their respective parts of the code that are only used inside this module
    • There is a Misc module that groups all the views (screens) that are not grouped into a module
  • Each directory under src except for modules must contain an index.js file that exports the directory's content. This is to simplify importing different parts of the code, and also to keep implementation details and internal structure of each directory hidden from its consumers
  • All the existing contexts are moved into a directory that best describes their purpose, and even some of them is renamed to better indicate that purpose rather than how they are implemented (like renaming amplitude context to cognito auth context)
  • As indicated in the structure description, the redux store was only used for error handling so it will be moved into globalError directory, and in the future it can be removed entirely and be replaced with a context that does the same thing
  • API is now renamed to services as it is used to call BE's API and not expose FE's API, so the name service has better description of its purpose and to make it less confusing
  • constants.js will be split into 2 parts (themes, and regions) and they will be in design and utils respectively

Migration Steps

First Step

To adopt the new structure by:

  • Moving all of (api, components, contexts, hooks, icons, store, theme) into the new structure that focuses on the purpose of each file as described above
  • Moving all the existing views to be under src/modules/Misc/views

Next N Steps

For the N modules we agree upon, we make the following:

  • Move the related views from src/modules/Misc/Views into src/modules/ModuleName/views
  • Move all of (components, hooks, services, constants) that are only used in the moved views (if any) to their respective directories (or files) under that module
  • Update src/router.js to use the new path of the views

Last Step

Finalize Misc module after all the other modules are decided and have been split by:

  • Moving (components, hooks, services, constants) that are only used in Misc module into Misc module

Files Naming Conventions

The current convention will be kept, and that is:

  • File names are camelCase unless otherwise is specified
  • Files that export a React component, like files for (components, contexts, icons, themes, and views) are named using PascalCase
  • Directory names are also in camelCase unless otherwise is specified
  • Module directory is named using PascalCase

Adding a New Module

In the future, if someone wants to add a new module, they should follow these steps:

  • Create a new directory for the new module under src/modules
  • The module structure should follow the same structure mentioned above
    • Mainly the module should be based around its views (or screens) so there must be a views directory
    • All of (components, hooks, services) should be under their respective directory in the module
  • Add the new module screens with their related URLs ot src/routes.js
  • In case they need to use a (component, hook, service) from another module, then that part need to refactored and moved into a common directory depending on its purpose (e.g. authentication, design, ...), and then both modules (the new one and the old one) should use that code from the common directory
  • Any utils or helper should be under src/utils unless it's a helper that is super specific to this module, then it can be in the same directory as the module under helpers or utils
  • In case they need to use any code from any common directory (any directory outside of src/modules) then no extra action is required

All created files and directories must follow the mentioned naming conventions

Notes

  • Most of the current components and hooks are indeed common between the different modules (or parts that we would call modules)
  • The views and APIs are already split into directories that loosely represent the modules that we are going to split them into, so splitting them should not be very hard
@nikpodsh
Copy link
Contributor

Thanks for sharing the plan for the migration!

I have a few questions for clarification:

  1. In the backend, we have this structure now: core, common and modules where core is a basic functionality needed for the app, and common some code shared between a few modules (not all). I am not sure if we need to follow the same in frontend, but I'd like just to bring this up for awareness.
  2. How a module will be integrated into the app? Say, I want to add a brand new module, where should I make changes to add my module to app?
  3. Just to confirm: the enabling/disabling modules is out of scope of this task and will be done in the other issue?

@AmrSaber
Copy link
Contributor Author

Hello @nikpodsh , thanks for the feedback, regarding the asked question:

  1. About the core module: I thought about it, and I think frontend is different from backend in that matter.This frontend is mainly used for calling the backend and there is no actual logic that happens on this side only, so I think there will be no need for the core part. If you think of a use case where we need a core model, please let me know.
  2. For adding a new module: you should create a new directory under src/modules with the specified conventions, and add its (components, hooks, and api) in that same directory, and in case you needed to use some code that is used in another module, you refactor that into common and both would use that from there -- thank you for mentioning this, I will add it to the planning
  3. Yes, this is only for the modularization, the enable/disable behaviour will be in a separate planning

@AmrSaber
Copy link
Contributor Author

I have updated the ticket with a section for adding a new module

@dlpzx
Copy link
Contributor

dlpzx commented Apr 3, 2023

Nice @AmrSaber
Here is my opinion on the discussion opened by @nikpodsh

  1. In my opinion Misc is very much what core is to the backend. Everything that cannot be modularized is basically core, so I would keep the same naming if possible
    1. So in this proposal we are just selecting all modules to be enabled and then later we would look at mechanisms to enabled or disable a feature right?

@AmrSaber
Copy link
Contributor Author

AmrSaber commented Apr 3, 2023

Hey @dlpzx , I have some comments on what you said...

  1. In the backend core refers to what must not be updated, and I think we agreed in a meeting before that we will have common in the backend as well. In the frontend on the other hand, misc is what cannot be classified, not necessarily a core feature though. Although they might seem similar in the effect, they are different in the meaning
  2. Yes, this planning only discusses the transition into modularized structure, and I will make another planning for the enabling/disabling feature

@AmrSaber
Copy link
Contributor Author

AmrSaber commented Apr 4, 2023

After consulting with Raul (Senior SDE) he had the following comments:

  • common with its current state, is an anti-pattern as for each of its directories, they describe the what and not the why
  • He suggested a different structure based on the why of each file and even some files renaming -- I will update the plan with those changes

@AmrSaber
Copy link
Contributor Author

AmrSaber commented Apr 5, 2023

I have updated the planning once again to reflect the latest feedback from Raul.

@dlpzx
Copy link
Contributor

dlpzx commented Apr 6, 2023

Hi @AmrSaber, thanks for the update.

  • I like the grouping under authentication and design, I think it is very clear for a developer on where to look for whenever doing changes.
  • The module globalErrors I am fine where it is, but I think it could also be in utils. I don't know if the error handling is a src-level module. But I see that the plan is to remove it entirely and add it in contexts, so maybe we can live with it.
  • I like that service matches BE service
  • The only thing that I am not convinced with is Misc, I think that when we get to a final list of "views that do not fit into any module" we can discuss more "whyish" names, miscelaneous is too open.

@AmrSaber
Copy link
Contributor Author

AmrSaber commented Apr 6, 2023

Hi @dlpzx I totally agree with you about the Misc it can mainly help us navigate through our transformation of the project, and later on we can think of a better naming for it, or think about a better place to put the views inside it. This will be clear to us once the modularization is done (i.e. all the modules that we know of).

Also, same thing about the globalErrors I think it's a bit lonely but as you said the plan is to remove it in the future and replace it with something else, and at that time we can think of a better structure for it (better place and different internal structure probably).

AmrSaber added a commit that referenced this issue Apr 18, 2023
### Feature or Bug-fix
<!-- please choose -->
- Feature

### Detail
- I am working as we agreed on the planning mentioned below
- First step I took is to unify all the exports and make most of them
into named exports (so it's easier to move things around and to have
each directory's export aggregated to be imported from its main
directory)
- I am trying to make the commit meaningful so that they can be reviewed
individually, but that does not always work, as there is a lot of moving
things around
- I created a branch out of `modularization-main` called
`modularization/frontend` and made it the target of this PR so that FE
updates are separated as we agreed

**Important note**: This PR does not contain any logic updates
whatsoever, only moving things around, and updating their export/import
with a minor exception of inlining the -previously- component `Topics`
which is now updated to be just an array and moved to constants.

### Relates
- #398

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Co-authored-by: Raul Cajias <[email protected]>
@dlpzx dlpzx added type: modularization Code refactoring project status: in-progress This issue has been picked and is being implemented priority: high labels Apr 18, 2023
AmrSaber added a commit that referenced this issue Apr 19, 2023
### Feature or Bugfix
- Refactoring

### Detail
- Modularize notebooks
- Move notebooks views into notebooks module
- Move services that were only used by notebooks into notebooks module
- Move notebooks components into notebooks module

### Relates
- #398

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
@dlpzx dlpzx closed this as completed Apr 26, 2023
maryamkhidir added a commit that referenced this issue Jul 21, 2023
### Feature or Bugfix
- Refactoring

### Detail
- Setup frontend (React) modules absolute imports using jsconfig.json

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
itsmo-amzn added a commit that referenced this issue Jul 25, 2023
### Feature or Bugfix
<!-- please choose -->
- Refactoring

### Detail
- create a 'shared' directory for shared components

### Relates
- #398 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
itsmo-amzn added a commit that referenced this issue Jul 25, 2023
### Feature or Bugfix
<!-- please choose -->
- Bugfix

### Detail
- compilation error when using absolute import

### Relates
- #398 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
maryamkhidir added a commit that referenced this issue Aug 8, 2023
### Feature or Bugfix

- Refactoring

### Detail
- Modularized Dashboards folder into views, components and services
subfolders
- Refactored relative imports used in the module to use absolute paths
- Removed unused scripts:
`services/graphql/Dashboard/createDashboard.js` and
`services/graphql/Dashboard/shareDashboard.js`
- Updated routes to reflect new file paths

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
maryamkhidir added a commit that referenced this issue Aug 9, 2023
### Feature or Bugfix
- Refactoring

### Detail
- Pipelines modularization into views, components and services
- Refactored relative imports used in the module to use absolute paths
- Removed unused scripts from services/graphql/Pipelines
- Updated routes to reflect new file paths

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
maryamkhidir added a commit that referenced this issue Aug 9, 2023
### Feature or Bugfix
<!-- please choose -->
- Refactoring

### Details
- Organizations modularization into views, components and services
- Refactored relative imports used in the module to use absolute paths
- Removed unused scripts from services/graphql/Organizations
- Updated routes to reflect new file paths

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
itsmo-amzn added a commit that referenced this issue Aug 10, 2023
### Feature or Bugfix
<!-- please choose -->
- Refactoring

### Detail
- Move folder views into Folders module
- Move services that were only used by Folders into Folders module
- Move Folders components into Folders module

### Relates
- [Frontend
modularization](#398)

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
maryamkhidir added a commit that referenced this issue Aug 10, 2023
### Feature or Bugfix
- Refactoring

### Detail
- Modularized worksheets folder into components, views and services
- Refactored relative imports used in the module to use absolute paths
- Removed unused scripts
- Updated routes to reflect new file paths
- Added module enablement option to routes and sidebar

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
maryamkhidir added a commit that referenced this issue Aug 10, 2023
### Feature or Bugfix
- Refactoring

### Detail
- Modularized Shares folder into components, views and services
- Refactored relative imports used in the module to use absolute paths
- Removed unused component: `ShareInboxTable.js`
- Updated routes to reflect new file paths

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
itsmo-amzn added a commit that referenced this issue Aug 10, 2023
### Feature or Bugfix
<!-- please choose -->
- Refactoring

### Detail
- fix linting issues in modules: Organizations, Dashboards, Pipelines

### Relates
- [Frontend
modularization](#398)

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
maryamkhidir added a commit that referenced this issue Aug 10, 2023
### Feature or Bugfix
- Refactoring

### Detail
- Tables folder modularization into views, components and services
- Refactored relative imports used in the module to use absolute paths
- Removed unused component
- Updated routes to reflect new file paths

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
itsmo-amzn added a commit that referenced this issue Aug 10, 2023
### Feature or Bugfix
<!-- please choose -->
- Refactoring

### Detail
- Align coding style in exporting components

### Relates
- [Frontend
modularization](#398)

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
itsmo-amzn added a commit that referenced this issue Aug 10, 2023
### Feature or Bugfix
<!-- please choose -->
- Refactoring

### Detail
- Move catalog views into catalog module
- Move services that were only used by catalog into catalog module
- Move catalogs components into catalog module

### Relates
- [Frontend
modularization](#398)

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
maryamkhidir added a commit that referenced this issue Aug 11, 2023
### Feature or Bugfix
- Refactoring

### Detail
- Modularise administration folder into components, views and services
- Refactored relative imports used in the module to use absolute paths
- Removed unused service: getUserRoleInTenant
- Updated routes to reflect new file paths

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
maryamkhidir added a commit that referenced this issue Aug 11, 2023
### Feature or Bugfix
- Refactoring

### Detail
- Cleanup imports to use absolute method

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
itsmo-amzn added a commit that referenced this issue Aug 14, 2023
### Feature or Bugfix
<!-- please choose -->
- Refactoring

### Detail
- Move Environments views into Environments module
- Move services that were only used by Environments into Environments
module
- Move Environments components into Environments module

### Relates
- [Frontend
modularization](#398)

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Co-authored-by: Nikita Podshivalov <[email protected]>
maryamkhidir added a commit that referenced this issue Aug 14, 2023
### Feature or Bugfix
- Refactoring

### Detail
- Modularized Glossaries folder into components, views and services
- Refactored relative imports used in the module to use absolute paths
- Removed unused scripts
- Updated routes to reflect new file paths

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
maryamkhidir added a commit that referenced this issue Aug 14, 2023
### Feature or Bugfix
- Refactoring

### Detail
- Moved Login view to the authentication module
- Refactored imports

### Relates
- #398

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
itsmo-amzn added a commit that referenced this issue Aug 15, 2023
### Feature or Bugfix
<!-- please choose -->
- Refactoring

### Detail
- Refactor module enablement

### Relates
- #398 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
itsmo-amzn added a commit that referenced this issue Aug 17, 2023
### Feature or Bugfix
<!-- please choose -->
- Merge request

### Detail
- Merge modularization-main into modularization/frontend 

### Relates
- #398 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Co-authored-by: Amr Saber <[email protected]>
Co-authored-by: Raul Cajias <[email protected]>
Co-authored-by: nikpodsh <[email protected]>
Co-authored-by: Maryam Khidir <[email protected]>
Co-authored-by: Nikita Podshivalov <[email protected]>
itsmo-amzn added a commit that referenced this issue Aug 21, 2023
### Feature or Bugfix
<!-- please choose -->
- Feature

### Detail
- module enablement for `datasets`, `catalog`, and `glossaries`

### Relates
- #398 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high status: in-progress This issue has been picked and is being implemented type: modularization Code refactoring project
Projects
None yet
Development

No branches or pull requests

3 participants