Align code style across the applicaton

[dcoa]

Review code style practices and code distribution.

Context:

1. For this issue 2 different styles on component definition was implemented aligning the code as explained here feat(authz): [FC-0099] add roles tab #7 (review)

2. Audit whether a component should be part of the app or a module component (e.g header, or toast context), however this was limited for the MVP scope, but need to be addressed for scaling the application in future stages.

[tonybusa]

I'm happy to pick up this issue and work on Context 1 above. I have a question on which declaration style we want to align on. The referenced link chooses React.FC. However, doing a search on the differences AI returns these other recommendations:

While React.FC is a valid way to declare functional components in TypeScript, the standard function declaration or arrow function expression are generally preferred in modern React development due to their conciseness, better type inference, and avoidance of potential issues related to implicit children props and generics.

Does anyone want to weigh in on the decision for this repo? I recommend the "standard function declaration" (rather than "arrow function expression").

[holaontiveros]

"Standar function declaration" feels a bit confusing... but I think I get were you are going but to be precise I would go with:

const ComponentName = ({ prop1, prop2}: ComponentNameProps) =>

because using something like FC<ComponentNameProps> feels to verbose for me without any benefits.

[jacobo-dominguez-wgu]

This post and the fact the react team is not using React.FC<> on their docs examples persuaded me to go with the standard way:

const App = ({ message }: AppProps) => <div>{message}</div>;

Another link for reference: https://react-typescript-cheatsheet.netlify.app/docs/basic/getting-started/function_components/

[jesusbalderramawgu]

the good thing about standard function declaration from my perspective is that is easier to understand in some occasions, but it is more verbose (I think this is more for previous versions).
arrow function declaration is more concise and cleaner.
I think in the latest versions of react or newer projects arrow functions are more common.
const MyComp = () =>{}

[tonybusa]

Seems there's a consensus to not use React.FC. However, I'm still curious on if there's a hard stance on "Standard Function Declaration":

    function MyComponent(props: { message: string, children?: React.ReactNode }) {
      return (
        <div>
          <h1>{props.message}</h1>
          {props.children}
        </div>
      );
    }

versus "Arrow Function Declaration":

    const MyComponent = ({ message, children }: { message: string, children?: React.ReactNode }) => {
      return (
        <div>
          <h1>{message}</h1>
          {children}
        </div>
      );
    };

The primary difference being arrow functions have lexical this binding, inheriting it from their parent scope at creation, while traditional functions determine this dynamically at invocation. Given that React uses props and hooks, I don't foresee an issue with using either but figured I'd mention it.

[dcoa]

@tonybusa The openedx standard is arrow declaration, you can check here in the eslint configuration

https://github.com/openedx/frontend-build/blob/4e8edbe6f2dbdd34c1a9c02be81832814defbdbd/config/.eslintrc.js#L38

[tonybusa]

In regards to Context 2 on this ticket: Audit whether a component should be part of the app or a module component (e.g header, or toast context), however this was limited for the MVP scope, but need to be addressed for scaling the application in future stages.

I've outlined the existing folder structure of the application (where "--" denotes a folder, and "-" a file):

-- src:
    -- authz-module
        -- components
            -- RoleCard
                - constants
                - messages
                - PermissionRow
            - AuthZLayout
            - AuthZTitle
            - PermissionTable
            - ResourceTooltip
        -- data
        -- libraries-manager
            -- components
                -- AddNewTeamMemberModal
                    - AddNewTeamMemberModal
                    - AddNewTeamMemberTrigger
                    - messages
                -- AssignNewRoleModal
                    - AssignNewRoleModal
                    - AssignNewRoleTrigger
                -- TeamTable
                    -- components
                        - MultipleChoiceFilter
                        - SearchFilter
                        - SortDropdown
                        - TableControlBar
                    -- hooks
                        - useQuerySettings
                    - messages 
                - ConfirmDeletionModal
                - messages
                - PublicReadToggle
            -- error page
            - LibrariesTeamManager
            - LibrariesUserManager
            - ToastManagerContext
    -- components
        - loading page
    -- data
        - api
        - hooks
        - utils
    -- i18n
    -- plugin-slots

And here's my proposed layout, which separates the structure into pages, and lifts any common components between those pages into it's own folder. Also every component would have it's own folder containing the .tsx and test.tsx files or any other reliant files i.e. messages.ts at the top-most common folder:

-- src
    -- common
        -- LoadingPage
        -- ErrorPage
        -- ToastManagerContext
        -- AuthZLayout
        -- AuthZTitle
        -- RoleCard
            -- PermissionRow
    -- pages 
        -- data # file:src/authz-module/data      
        -- LibrariesTeamManager # URL:/admin-console/authz/libraries/{library:id}
            -- PublicReadToggle
            -- AddNewTeamMemberModal
                -- AddNewTeamMemberTrigger
            -- Components
                -- TeamTable
                    -- hooks
                    -- TableControlBar
                        -- SortDropdown
                        -- SearchFilter
                        -- MultipleChoiceFilter
                -- PermissionTable
                    -- ResourceTooltip
        -- LibrariesUserManager # URL:/admin-console/authz/libraries/{library:id}/{user:id}
            -- AssignNewRoleModal
                -- AssignNewRoleTrigger
            -- ConfirmDeletionModal
    -- data
        - api
        - hooks
        - utils
    -- i18n
    -- plugin-slots

To further answer the question around "modular" components, these are that I believe could be reusable:

-- LoadingPage
-- ErrorPage
-- ToastManagerContext
-- RoleCard
-- PermissionRow
-- SortDropdown
-- SearchFilter
-- MultipleChoiceFilter

[dcoa]

Thank you, I will have a look of it in my comeback, so please don't worry about the second topic for now :)