search

openfun / jitsi-magnify

An authentication and room management system for Jitsi built with Django/React
MIT License
23 stars 6 forks source link

Structure of the repo #6

Closed Leonils closed 2 years ago

Leonils commented 2 years ago

Feature Request

Is your feature request related to a problem or unsupported use case? Please describe. Currently, this repository is built around a single django infrastructure. As we might want to add a frontend and to extract some logic to libraries, we would need a more open structure.

Describe the solution you'd like I suggest to create a monorepo structure, with at least 6 parts:

For the javascript part, we can use yarn workspace feature to declare, import, and manage libraries without using too much relative links.

Describe alternatives you've considered Another possible solution would be to create 4 different repository. It would allow clearer separation between libs and demos, and between python and javascript code, but would slightly slow down the development process, as it would need to split it through 4 different repositories, with potentially delays between them.

Discovery, Documentation, Adoption, Migration Strategy The whole structure would be explained in a documentation in the /documentation folder

Do you want to work on it through a Pull Request? Yes

Points of discussion

Tools As this issue is a kind of "init project" one, it should include installation and setup of tools. For the front-end, I suggest to start with

DarkPitoune commented 2 years ago

I recommend using react-query to create hooks to fetch on the back end. It reduces boilerplate code to handle status, error and loading state and on-success function. It's easy to pick up and drives us toward a structure where all the API calls are stored in a single folder.

sampaccoud commented 2 years ago

We should definitely keep a monorepo. For the documentation, our practice is to name the directory /docs.

For the library, maybe we should use the term apps to fit the Django world. Also I don't think we need to repeat -lib in the children directories. So maybe:

└── src/
    ├── apps/
    │   ├── backend
    │   └── frontend
    └── demo/
        ├── backend
        └── frontend

I propose to publish the library part as one package with several apps. We would have a magnify package with, if necessary magnify.core, magnify.rooms, magnify.calendar, etc. Let's not make several apps unless we really think it's necessary. You can refer to our project https://github.com/openfun/richie to see how we build and publish Django and React packages to pypi and npm respectively with CircleCI.

+1 for the frontend dependencies you proposed.

Leonils commented 2 years ago

In a first time, as the model presented in issue #5 is strongly intricated, we should indeed create a single backend app, but we may name it with a meaningful name, as room-management.

For the frontend however, not sure we will expose an entire app. We may mainly exports components, data-access hooks, interfaces, and views. Maybe we can go with something like : 

└── src/
    ├── apps/
    │   └── room-management
    ├── front-libs/
    │   ├── .storybook
    │   ├── components
    │   ├── data-access
    │   ├── feature-1 
    │   ├── feature-2 
    │   └── feature-3 
    └── demo/
        ├── backend
        └── frontend
lunika commented 2 years ago

As suggested by @sampaccoud you should look of what is made on richie and use the same structure. I don't think you need to split the front application with multiple sub directories. In feature-1 you will probably need components and other shared code. You plan to have more than one front package ?

sampaccoud commented 2 years ago

Ok then should we first split on front/back?

└── src/
    ├── backend/
    │   ├── apps
    │   └── demo
    └── frontend/
        ├── .storybook
        ├── components
        ├── data-access
        ├── feature-1
        ├── feature-2
        ├── feature-3
        └── demo

let's rename room-management to just rooms. As you can see in richie, we choose to use the plural version of things. We can discuss this again in the near future :wink:

Leonils commented 2 years ago

In richie we have a structure that simplifies to :

└── src/
    ├── frontend
    └── richie/
        ├── apps
        │   ├── core
        │   ├── courses
        │   ├── demo
        │   └── search
        └── plugins

And the front-end seems to expose an index.tsx providing the whole application. Am I right ?

In our case, we may try to expose more directly the components and the views to the users of the front-end lib to help the community to reuse them.

About the structure, it's open. We can indeed create very cohesive front end libs, with some shared components in a specific lib, or create a single lib with everything inside. This is made for instance by babel, testing-lib, ... I have not strong opinion about that.

I like the idea of demo being grouped, so may be

└── src/
    ├── backend-apps/
    │   └── rooms
    ├── frontend-lib/
    │    ├── .storybook
    │    ├── components
    │    ├── data-access
    │    ├── feature-1
    │    ├── feature-2
    │    └── feature-3
    └── demo/
        ├── backend
        └── frontend

With frontend-lib beeing a single publishable package

sampaccoud commented 2 years ago

Yes your understanding of the relation between the frontend and backend in richie is correct. This is probably a good pattern for manify as well. In richie, developpers can also reuse components if they install the npm library that we publish. Probably a good pattern for magnify as well? Ok for your last proposal, just maybe avoid 2-words names for simple directories and do something like this:

└── src/
    ├── backend/
    │   └── rooms
    ├── frontend/
    │    ├── .storybook
    │    ├── components
    │    ├── data-access
    │    ├── feature-1
    │    ├── feature-2
    │    └── feature-3
    └── demo/
        ├── backend
        └── frontend

The backend and frontend packages should be published to pypi and npm respectively under the name magnify

Leonils commented 2 years ago

After working on it, I have some other considerations about the structure. Right now, I have got a working draft with the following structure: 

└── src/
    ├── demo/
    │   ├── backend
    │   └── frontend
    ├── frontend/
    │    ├── .storybook
    │    ├── components
    │    ├── data-access
    │    ├── feature-1
    │    ├── feature-2
    │    └── feature-3
    ├── magnify/
    │    ├── rooms
    │    └── settings.py
    └── tests/
        └── apps
            └── rooms

I moved files from the previous django project into the rooms app, tests, and demo/backend. Right now the demo app works and the tests pass.

Some issues to consider :

Yarn workspaces WIth yarn workspace, we can define a workspace ("@openfun" for instance), and libraries are called "@openfun/frontend-demo", "@openfun/frontend-lib". This allow to import "@openfun/frontend-lib" in the monorepo, and publish packages independently.

sampaccoud commented 2 years ago

Not a big fan to add openfun to the name but we should probably add jitsi in the name? jitsi-magnify? For the backend, either leave it like this (actually that's what we have in richie as well :man_shrugging: ) or add one more subdirectory?

Leonils commented 2 years ago

Thank you for the feedback. So we may name the workspace "jitsi-magnify", and then the python backend package "jitsi-magnify", the frontend package "@jitsi-magnify/frontend-lib" and the frontend demo "@jitsi-magnify/frontend-demo". Not sure if we need to publish the django sandbox (demo) as a pipy package

Some update about the frontend

I have now the previous structure almost working. 

└── src/
    ├── demo/
    │   ├── backend
    │   └── frontend
    │       ├── src
    │       │    └── App.tsx  // display a button from the frontend lib     
    │       └── package.json  // can be published as @jitsi-magnify/frontend-demo, or built and serve from CDN
    ├── frontend/
    │    ├── .storybook
    │    ├── components    // exports a single TestButton
    │    └── package.json   // can be published as @jisty-magnify/frontend-lib
    ├── magnify/
    │    ├── rooms
    │    └── settings.py
    ├── package.json   // workspace definition
    └── tests/
        └── apps
            └── rooms

Theses features should work, (to review tomorrow ?)

Do you see anything else that need to work before going on ? I didn't make the publishing part right now, but it should be smooth as every folder is either a python or a node package.

jbpenrath commented 2 years ago

Use of namespace in frontend package name is pretty common today (next, testing-library, babel, formatjs, apollo... use this convention). In addition to prevent package name conflicts, this also makes easier to search package on npm (e.g: https://www.npmjs.com/search?q=%40openfun).

About the name of the frontend package, I'm not a fan of frontend-lib, frontend-demo. In my opinion, frontend is useless here as we are using this codebase in a frontend context. Why not replicate naming convention of django apps ?

Leonils commented 2 years ago

Thank you for the input ! Are we sure we won't have backend packages in javascript soon or later ? If we are sure, so we can commit to that indeed. Else, what would be the naming then ? Maybe it's wrong to think about that so soon in the project development, but package renaming is so painful...