From ea205f3bee416fa8042d7517828e3ba2558cb014 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 15 Dec 2023 10:57:58 -0500 Subject: [PATCH 1/9] update client arch docs for team-owned libs --- custom-words.txt | 1 + docs/architecture/clients/index.md | 52 +++++++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/custom-words.txt b/custom-words.txt index 668bef9f..01f2bf48 100644 --- a/custom-words.txt +++ b/custom-words.txt @@ -5,6 +5,7 @@ Bitwarden bytemark CODEOWNERS +codeownership CQRS dockerized Gitter diff --git a/docs/architecture/clients/index.md b/docs/architecture/clients/index.md index 6a53e495..fe274783 100644 --- a/docs/architecture/clients/index.md +++ b/docs/architecture/clients/index.md @@ -15,13 +15,55 @@ The mono-repository root directory contains three main folders. - `bitwarden_license` - Bitwarden Licensed version of the web vault. - `libs` - Shared code between the different applications. -`libs` contains the following projects. +## Libs -- `Common` - Common code shared between all the clients including CLI. -- `Angular` - Angular specific code used by all the visual clients. -- `Components` - Angular Components Library. -- `Node` - Used to be shared code for CLI and Directory Connector CLI, but since directory connector +Historically, much of the feature logic of Bitwarden has existed within the `apps` directories, only +being moved to `libs` (usually `libs/common` or `libs/angular`) if it needed to be shared between +multiple clients. + +We are attempting to move to a more modular architecture by creating additional libs that are more +feature-focused and more accurately convey team-codeownership. + +- `common` - Common code shared between all the clients including CLI. +- `angular` - Low-level Angular specific code used by all the visual clients. Code that is more tied + to Bitwarden business logic should be moved to a team-owned lib. +- `node` - Used to be shared code for CLI and Directory Connector CLI, but since directory connector no longer uses the same version of libs this module is deprecated. +- `admin-console` - Code owned by the Admin Console team. +- `auth` - Code owned by the Auth team. +- `billing` - Code owned by the Billing team. +- `components` - Angular implementation of the Bitwarden design system. + [See more.](https://components.bitwarden.com/) +- `exporter` - Code related to exporting; owned by the Tools team. +- `importer` - Code related to importing; owned by the Tools team. +- `platform` - Code owned by the Platform team. +- `vault` - Code owned by the Vault team. + +For more on this approach, check out the +[Nx documentation](https://nx.dev/concepts/more-concepts/applications-and-libraries): + +> place 80% of your logic into the `libs/` folder and 20% into `apps/` We are doing the opposite of +> this right now. What code should be moved from `apps/` to `libs/`? + +- At this time, any code in the `/apps/web/*` that doesn't have a tight coupling with the web client + can be moved into a lib. For components, this means they should only be the CL, no global CSS or + anything exported from web-specific modules. + +> Having a dedicated library project is a much stronger boundary compared to just separating code +> into folders, though. Each library has a so-called "public API", represented by an index.ts barrel +> file. This forces developers into an "API thinking" of what should be exposed and thus be made +> available for others to consume, and what on the others side should remain private within the +> library itself. + +An existing example of this pattern is `@bitwarden/components`: + +- It is used by multiple apps and other libs +- It manages a clear public and private API boundary through its barrel file. +- Codeownership is clear--only the files within `libs/components` +- The team can make independent decisions around internal folder structure and code style. + +As part of this process, we are also investigating utilizing additional tooling (such as Nx) to make +creating new libs on the fly easier. ## Package Diagram From ae366864ae374552dc0ba078e863a21a6ef947f4 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 15 Dec 2023 11:05:46 -0500 Subject: [PATCH 2/9] update formatting --- docs/architecture/clients/index.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/architecture/clients/index.md b/docs/architecture/clients/index.md index fe274783..0cd5309e 100644 --- a/docs/architecture/clients/index.md +++ b/docs/architecture/clients/index.md @@ -42,12 +42,12 @@ feature-focused and more accurately convey team-codeownership. For more on this approach, check out the [Nx documentation](https://nx.dev/concepts/more-concepts/applications-and-libraries): -> place 80% of your logic into the `libs/` folder and 20% into `apps/` We are doing the opposite of -> this right now. What code should be moved from `apps/` to `libs/`? +> place 80% of your logic into the `libs/` folder and 20% into `apps/` -- At this time, any code in the `/apps/web/*` that doesn't have a tight coupling with the web client - can be moved into a lib. For components, this means they should only be the CL, no global CSS or - anything exported from web-specific modules. +We are doing the opposite of this right now. What code should be moved from `apps/` to `libs/`? At +this time, any code in the `/apps/web/*` that doesn't have a tight coupling with the web client can +be moved into a lib. For components, this means they should only be the CL, no global CSS or +anything exported from web-specific modules. > Having a dedicated library project is a much stronger boundary compared to just separating code > into folders, though. Each library has a so-called "public API", represented by an index.ts barrel From 1603713e09f0bca5c6f0f1d70eab11b577f7d1f9 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 15 Dec 2023 11:11:45 -0500 Subject: [PATCH 3/9] fix code ownership spelling --- custom-words.txt | 1 - docs/architecture/clients/index.md | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/custom-words.txt b/custom-words.txt index 01f2bf48..668bef9f 100644 --- a/custom-words.txt +++ b/custom-words.txt @@ -5,7 +5,6 @@ Bitwarden bytemark CODEOWNERS -codeownership CQRS dockerized Gitter diff --git a/docs/architecture/clients/index.md b/docs/architecture/clients/index.md index 0cd5309e..89300333 100644 --- a/docs/architecture/clients/index.md +++ b/docs/architecture/clients/index.md @@ -22,7 +22,7 @@ being moved to `libs` (usually `libs/common` or `libs/angular`) if it needed to multiple clients. We are attempting to move to a more modular architecture by creating additional libs that are more -feature-focused and more accurately convey team-codeownership. +feature-focused and more accurately convey team code ownership. - `common` - Common code shared between all the clients including CLI. - `angular` - Low-level Angular specific code used by all the visual clients. Code that is more tied @@ -59,7 +59,7 @@ An existing example of this pattern is `@bitwarden/components`: - It is used by multiple apps and other libs - It manages a clear public and private API boundary through its barrel file. -- Codeownership is clear--only the files within `libs/components` +- Code ownership is clear--only the files within `libs/components` - The team can make independent decisions around internal folder structure and code style. As part of this process, we are also investigating utilizing additional tooling (such as Nx) to make From f55c323355833e007c6a193ad558db271d328e9e Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 15 Dec 2023 13:31:20 -0500 Subject: [PATCH 4/9] consistent list formatting --- docs/architecture/clients/index.md | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/docs/architecture/clients/index.md b/docs/architecture/clients/index.md index 89300333..0af1813d 100644 --- a/docs/architecture/clients/index.md +++ b/docs/architecture/clients/index.md @@ -24,20 +24,19 @@ multiple clients. We are attempting to move to a more modular architecture by creating additional libs that are more feature-focused and more accurately convey team code ownership. -- `common` - Common code shared between all the clients including CLI. -- `angular` - Low-level Angular specific code used by all the visual clients. Code that is more tied - to Bitwarden business logic should be moved to a team-owned lib. -- `node` - Used to be shared code for CLI and Directory Connector CLI, but since directory connector - no longer uses the same version of libs this module is deprecated. -- `admin-console` - Code owned by the Admin Console team. -- `auth` - Code owned by the Auth team. -- `billing` - Code owned by the Billing team. -- `components` - Angular implementation of the Bitwarden design system. +- `common`: Common code shared between all the clients including CLI +- `angular`: Low-level Angular specific utilities used by all the visual clients +- `node`: Used to be shared code for CLI and Directory Connector CLI, but since directory connector + no longer uses the same version of libs this module is deprecated +- `admin-console`: Code owned by the Admin Console team +- `auth`: Code owned by the Auth team +- `billing`: Code owned by the Billing team +- `components`: Angular implementation of the Bitwarden design system [See more.](https://components.bitwarden.com/) -- `exporter` - Code related to exporting; owned by the Tools team. -- `importer` - Code related to importing; owned by the Tools team. -- `platform` - Code owned by the Platform team. -- `vault` - Code owned by the Vault team. +- `exporter`: Code related to exporting; owned by the Tools team +- `importer`: Code related to importing; owned by the Tools team +- `platform`: Code owned by the Platform team +- `vault`: Code owned by the Vault team For more on this approach, check out the [Nx documentation](https://nx.dev/concepts/more-concepts/applications-and-libraries): @@ -58,9 +57,9 @@ anything exported from web-specific modules. An existing example of this pattern is `@bitwarden/components`: - It is used by multiple apps and other libs -- It manages a clear public and private API boundary through its barrel file. +- It manages a clear public and private API boundary through its barrel file - Code ownership is clear--only the files within `libs/components` -- The team can make independent decisions around internal folder structure and code style. +- The team can make independent decisions around internal folder structure and code style As part of this process, we are also investigating utilizing additional tooling (such as Nx) to make creating new libs on the fly easier. From 8ce7a11d3657b49f87303d5efcf2df87e1c6111f Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 15 Dec 2023 14:05:05 -0500 Subject: [PATCH 5/9] apply code review --- docs/architecture/clients/index.md | 32 ++++++++++++++++++------------ 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/docs/architecture/clients/index.md b/docs/architecture/clients/index.md index 0af1813d..ecaf3bf2 100644 --- a/docs/architecture/clients/index.md +++ b/docs/architecture/clients/index.md @@ -17,15 +17,15 @@ The mono-repository root directory contains three main folders. ## Libs -Historically, much of the feature logic of Bitwarden has existed within the `apps` directories, only -being moved to `libs` (usually `libs/common` or `libs/angular`) if it needed to be shared between -multiple clients. +Historically, much of the feature logic of Bitwarden has existed within the `apps/` directories, +only being moved to `libs/` (usually `libs/common/` or `libs/angular/`) if it needed to be shared +between multiple clients. We are attempting to move to a more modular architecture by creating additional libs that are more -feature-focused and more accurately convey team code ownership. +feature-focused and which more accurately convey team code ownership. - `common`: Common code shared between all the clients including CLI -- `angular`: Low-level Angular specific utilities used by all the visual clients +- `angular`: Low-level Angular utilities used by all the visual clients - `node`: Used to be shared code for CLI and Directory Connector CLI, but since directory connector no longer uses the same version of libs this module is deprecated - `admin-console`: Code owned by the Admin Console team @@ -38,15 +38,21 @@ feature-focused and more accurately convey team code ownership. - `platform`: Code owned by the Platform team - `vault`: Code owned by the Vault team -For more on this approach, check out the +### Background + +More on this approach can be found in the [Nx documentation](https://nx.dev/concepts/more-concepts/applications-and-libraries): > place 80% of your logic into the `libs/` folder and 20% into `apps/` -We are doing the opposite of this right now. What code should be moved from `apps/` to `libs/`? At -this time, any code in the `/apps/web/*` that doesn't have a tight coupling with the web client can -be moved into a lib. For components, this means they should only be the CL, no global CSS or -anything exported from web-specific modules. +We are doing the opposite of this right now. The code that remains in `apps/` should be primarily +concerned with bootstrapping the application and consuming and configuring code exported from +`libs/`. Over time, code not meeting this criteria should be moved from `apps/` to `libs/`. + +Any code in the `apps/` that doesn't have a tight coupling with the client can be moved into a lib: +if an exported member is wholly dependent on other libs but not `apps/`, it can likely be moved to a +lib itself. (More concretely, this bars components that rely on global client-specific CSS, but not +components built with Tailwind and the CL.) > Having a dedicated library project is a much stronger boundary compared to just separating code > into folders, though. Each library has a so-called "public API", represented by an index.ts barrel @@ -56,10 +62,10 @@ anything exported from web-specific modules. An existing example of this pattern is `@bitwarden/components`: -- It is used by multiple apps and other libs +- It is consumed by multiple apps and other libs - It manages a clear public and private API boundary through its barrel file -- Code ownership is clear--only the files within `libs/components` -- The team can make independent decisions around internal folder structure and code style +- Code ownership is clear--the CL team only owns the files within `libs/components` +- The CL team can make independent decisions around internal folder structure and code style As part of this process, we are also investigating utilizing additional tooling (such as Nx) to make creating new libs on the fly easier. From dc3a13704e996c25a17d2e5816c180acb01358f6 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 15 Dec 2023 14:21:31 -0500 Subject: [PATCH 6/9] add clarity to global css example; fix formatting --- docs/architecture/clients/index.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/architecture/clients/index.md b/docs/architecture/clients/index.md index ecaf3bf2..e520ce40 100644 --- a/docs/architecture/clients/index.md +++ b/docs/architecture/clients/index.md @@ -10,10 +10,10 @@ and a single [Git repository](https://github.com/bitwarden/clients). The mono-repository root directory contains three main folders. -- `apps` - Our different application specific code, consists of `web`, `browser`, `desktop` and +- `apps`: Our different application specific code, consists of `web`, `browser`, `desktop` and `cli`. -- `bitwarden_license` - Bitwarden Licensed version of the web vault. -- `libs` - Shared code between the different applications. +- `bitwarden_license`: Bitwarden Licensed version of the web vault. +- `libs`: Shared code between the different applications. ## Libs @@ -51,8 +51,8 @@ concerned with bootstrapping the application and consuming and configuring code Any code in the `apps/` that doesn't have a tight coupling with the client can be moved into a lib: if an exported member is wholly dependent on other libs but not `apps/`, it can likely be moved to a -lib itself. (More concretely, this bars components that rely on global client-specific CSS, but not -components built with Tailwind and the CL.) +lib itself. (More concretely, this bars components that rely on global client-specific CSS — but not +components built with Tailwind and the CL — from being included in a lib) > Having a dedicated library project is a much stronger boundary compared to just separating code > into folders, though. Each library has a so-called "public API", represented by an index.ts barrel @@ -64,7 +64,7 @@ An existing example of this pattern is `@bitwarden/components`: - It is consumed by multiple apps and other libs - It manages a clear public and private API boundary through its barrel file -- Code ownership is clear--the CL team only owns the files within `libs/components` +- Code ownership is clear — the CL team only owns the files within `libs/components` - The CL team can make independent decisions around internal folder structure and code style As part of this process, we are also investigating utilizing additional tooling (such as Nx) to make From bfba1fe8400400da14f030afc7b9bc485e3b8742 Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 9 Jan 2024 09:56:13 -0500 Subject: [PATCH 7/9] add team-owned libs adr --- docs/architecture/adr/0022-team-owned-libs.md | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 docs/architecture/adr/0022-team-owned-libs.md diff --git a/docs/architecture/adr/0022-team-owned-libs.md b/docs/architecture/adr/0022-team-owned-libs.md new file mode 100644 index 00000000..f1918cb3 --- /dev/null +++ b/docs/architecture/adr/0022-team-owned-libs.md @@ -0,0 +1,46 @@ +--- +adr: "0022" +status: In progress +date: 2023-01-09 +tags: [clients] +--- + +# 0001 - Team-owned libs + + + +## Context and Problem Statement + +Historically, much of the feature logic of Bitwarden has existed within the `apps/` directories, +only being moved to `libs/` (usually `libs/common/` or `libs/angular/`) if it needed to be shared +between multiple clients. + +This pattern makes it more difficult for teams to independently manage code they own. + +## Considered Options + +- **Do nothing** - Keep our current pattern. +- **Template-driven forms** - Break functionality out of `apps/`, `libs/common/`, and + `libs/angular/` and move into libs that are smaller, feature-focused, and owned by a team. + +Source: https://angular.io/guide/forms-overview#choosing-an-approach + +## Decision Outcome + +Chosen option: **Team-owned libs** + +### Positive Consequences + +- **Clear public API** - Each team can decide what is public and private within their library. This + prevents teams from relying on code that is meant to be internal. +- **Faster builds** - +- **More team autonomy** - +- **Easier to share code between apps** - +- **Clearer dependency graph** - It will be easier to inspect which teams are relying internal and + external dependencies. +- **Simplified code ownership** - + +### Negative Consequences + +- **Managing circular dependencies** - +- **More boilerplate** - From 11f0e6240212bb49720d0feece929a4eedcb5a49 Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 9 Jan 2024 10:19:29 -0500 Subject: [PATCH 8/9] update adr --- docs/architecture/adr/0022-team-owned-libs.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/architecture/adr/0022-team-owned-libs.md b/docs/architecture/adr/0022-team-owned-libs.md index f1918cb3..64d31976 100644 --- a/docs/architecture/adr/0022-team-owned-libs.md +++ b/docs/architecture/adr/0022-team-owned-libs.md @@ -5,7 +5,7 @@ date: 2023-01-09 tags: [clients] --- -# 0001 - Team-owned libs +# 0022 - Team-owned libs @@ -33,12 +33,12 @@ Chosen option: **Team-owned libs** - **Clear public API** - Each team can decide what is public and private within their library. This prevents teams from relying on code that is meant to be internal. -- **Faster builds** - - **More team autonomy** - - **Easier to share code between apps** - - **Clearer dependency graph** - It will be easier to inspect which teams are relying internal and external dependencies. - **Simplified code ownership** - +- **Incremental builds** - ### Negative Consequences From b09ccc728951255754d34a1f20341f383d63e93c Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 9 Jan 2024 10:30:56 -0500 Subject: [PATCH 9/9] update adr --- custom-words.txt | 1 + docs/architecture/adr/0022-team-owned-libs.md | 23 ++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/custom-words.txt b/custom-words.txt index 668bef9f..6ac7d3db 100644 --- a/custom-words.txt +++ b/custom-words.txt @@ -14,6 +14,7 @@ hotfixes inet IntelliJ Iterm +incentivizing jslib jumpcloud keychain diff --git a/docs/architecture/adr/0022-team-owned-libs.md b/docs/architecture/adr/0022-team-owned-libs.md index 64d31976..f9c6493f 100644 --- a/docs/architecture/adr/0022-team-owned-libs.md +++ b/docs/architecture/adr/0022-team-owned-libs.md @@ -20,10 +20,27 @@ This pattern makes it more difficult for teams to independently manage code they ## Considered Options - **Do nothing** - Keep our current pattern. -- **Template-driven forms** - Break functionality out of `apps/`, `libs/common/`, and - `libs/angular/` and move into libs that are smaller, feature-focused, and owned by a team. +- **Template-driven forms** - Move functionality out of `apps/`, `libs/common/`, and `libs/angular/` + and move into libs that are smaller, feature-focused, and owned by a team. -Source: https://angular.io/guide/forms-overview#choosing-an-approach +This is informed by [Nx](https://nx.dev/concepts/more-concepts/applications-and-libraries), a +monorepo tool: + +> A typical Nx workspace is structured into "apps" and "libs". This distinction allows us to have a +> more modular architecture by following a separation of concerns methodology, incentivizing the +> organization of our source code and logic into smaller, more focused and highly cohesive units. + +
+A common mental model is to see the application as "containers" that link, bundle and compile functionality implemented in libraries for being deployed. As such, if we follow a 80/20 approach: + + - place 80% of your logic into the libs/ folder + - and 20% into apps/ + +Note, these libraries don’t necessarily need to be built separately, but are rather consumed and +built by the application itself directly. Hence, nothing changes from a pure deployment point of +view. + +
## Decision Outcome