[CL-553] Migrate CL to Control Flow syntax #12390

Hinton · 2024-12-13T13:36:46Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/CL-553

📔 Objective

Explore how the new control flow syntax looks and behaves in a limited scope (i.e. Component Library).

📸 Screenshots

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

github-actions · 2024-12-13T13:44:55Z

Checkmarx One – Scan Summary & Details – 2360c81d-72d0-4bb2-a0ea-676b63fc1a19

New Issues (6)

Checkmarx found the following issues in this Pull Request

Issue	Source File / Package	Checkmarx Insight
Angular_Improper_Type_Pipe_Usage	/apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.html: 21	details The value used in showBadge$ at /apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.html in line 21 may throw ... Attack Vector
Angular_Improper_Type_Pipe_Usage	/apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.html: 14	details The value used in buttonSupportingText$ at /apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.html in line 14... Attack Vector
Angular_Improper_Type_Pipe_Usage	/apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.html: 14	details The value used in buttonSupportingText$ at /apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.html in line 14... Attack Vector
Angular_Improper_Type_Pipe_Usage	/apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.html: 26	details The value used in numberOfAppliedFilters$ at /apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.html in line ... Attack Vector
Client_Privacy_Violation	/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.html: 19	details Method weak_passwords_report_component at line 19 of /apps/web/src/app/tools/reports/pages/weak-passwords-report.component.html sends user informat... Attack Vector
Missing_HSTS_Header	/libs/common/src/services/api.service.ts: 243	details The web-application does not define an HSTS header, leaving it vulnerable to attack. Attack Vector

Fixed Issues (9)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	Angular_Improper_Type_Pipe_Usage	/libs/components/src/navigation/nav-divider.component.html: 1
	Client_Privacy_Violation	/libs/tools/generator/components/src/username-generator.component.html: 3
	Client_Privacy_Violation	/libs/tools/generator/components/src/credential-generator-history.component.html: 3
	Client_Privacy_Violation	/libs/vault/src/components/password-history-view/password-history-view.component.html: 6
	Client_Privacy_Violation	/apps/web/src/app/tools/password-generator-history.component.html: 11
	Client_Privacy_Violation	/libs/vault/src/components/password-history-view/password-history-view.component.html: 6
	Client_Privacy_Violation	/apps/web/src/app/tools/password-generator-history.component.html: 11
	Client_Privacy_Violation	/libs/components/src/color-password/color-password.component.ts: 31
	Client_Privacy_Violation	/libs/components/src/color-password/color-password.component.ts: 17

codecov · 2024-12-13T13:47:46Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 35.30%. Comparing base (331c04a) to head (e9e3b66).
Report is 50 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12390      +/-   ##
==========================================
- Coverage   35.31%   35.30%   -0.02%     
==========================================
  Files        2998     2998              
  Lines       90950    90937      -13     
  Branches    16981    16981              
==========================================
- Hits        32123    32103      -20     
- Misses      56335    56342       +7     
  Partials     2492     2492

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…-flow

Hinton · 2025-01-07T17:16:17Z

@willmartian it seems the migration was unable to convert libs/components/src/navigation/nav-item.component.html and we will need to manually migrate it.

Hinton · 2025-01-07T17:21:17Z

libs/components/src/chip-select/chip-select.component.ts

@@ -44,6 +44,7 @@ export type ChipSelectOption<T> = Option<T> & {
      multi: true,
    },
  ],
+  preserveWhitespaces: false,


We needed to disable this on a few components. Platform is exploring disabling this globally.

…-flow

willmartian

I am really liking the new control flow. Really improves grok-ability of what is going on in a template. Also, gets rid of ng-container, which always felt a bit weird to me.

Gonna follow this PR up with enabling @angular-eslint/template/prefer-control-flow in libs/components (https://bitwarden.atlassian.net/browse/CL-559)

willmartian · 2025-01-10T15:23:15Z

libs/components/src/button/button.stories.ts

Cool that the migrator could link this if/else even though we were not already using NgIf's else syntax.

Nah, I manually migrated this. The migrator doesn't work in storybook.

willmartian · 2025-01-10T15:33:52Z

@willmartian it seems the migration was unable to convert libs/components/src/navigation/nav-item.component.html and we will need to manually migrate it.

We can do that in a followup; created issue here: https://bitwarden.atlassian.net/browse/CL-558

willmartian · 2025-01-10T15:35:57Z

Holding until after next RC cut.

…-flow # Conflicts: # libs/components/src/dialog/simple-dialog/simple-configurable-dialog/simple-configurable-dialog.service.stories.ts

…-flow # Conflicts: # libs/components/src/dialog/dialog/dialog.component.html # libs/components/src/form-field/form-field.component.html # libs/components/src/layout/layout.component.html

Hinton · 2025-02-03T19:11:49Z

Merging, will be tested on main.

Hinton requested a review from a team as a code owner December 13, 2024 13:36

Hinton requested a review from willmartian December 13, 2024 13:36

Hinton marked this pull request as draft December 13, 2024 13:37

willmartian requested a review from vleague2 December 13, 2024 13:39

Base automatically changed from arch/standalone-components to main December 17, 2024 22:29

Hinton added 2 commits December 20, 2024 10:52

Run ng generate @angular/core:control-flow

d95218f

Fix errors

19bd4b8

Hinton force-pushed the arch/control-flow branch from 0f190d5 to 19bd4b8 Compare December 20, 2024 09:53

Hinton added 2 commits January 7, 2025 17:51

Merge branch 'main' of github.com:bitwarden/clients into arch/control…

3b5c5d9

…-flow

Re-run migration. to clean up some imports and manually convert story.

30cd36b

Hinton marked this pull request as ready for review January 7, 2025 17:15

Hinton changed the title ~~Explore control flow syntax~~ [CL-553] Migrate CL to Control Flow syntax Jan 7, 2025

Hinton commented Jan 7, 2025

View reviewed changes

Hinton and others added 5 commits January 7, 2025 18:44

Fix build

b9aeae1

Fix tests

f364f09

Merge branch 'main' of github.com:bitwarden/clients into arch/control…

41a09ae

…-flow

Fix nav group

1ecd73a

Merge branch 'main' into arch/control-flow

c2373d1

willmartian previously approved these changes Jan 10, 2025

View reviewed changes

willmartian added the hold do not merge, do not approve yet label Jan 10, 2025

Merge branch 'main' of github.com:bitwarden/clients into arch/control…

0cc2509

…-flow # Conflicts: # libs/components/src/dialog/simple-dialog/simple-configurable-dialog/simple-configurable-dialog.service.stories.ts

Hinton dismissed willmartian’s stale review via 0cc2509 January 27, 2025 13:56

Hinton requested a review from willmartian January 27, 2025 13:58

Hinton added needs-qa Marks a PR as requiring QA approval and removed hold do not merge, do not approve yet labels Jan 27, 2025

willmartian previously approved these changes Jan 27, 2025

View reviewed changes

Merge branch 'main' of github.com:bitwarden/clients into arch/control…

e9e3b66

…-flow # Conflicts: # libs/components/src/dialog/dialog/dialog.component.html # libs/components/src/form-field/form-field.component.html # libs/components/src/layout/layout.component.html

Hinton dismissed willmartian’s stale review via e9e3b66 January 28, 2025 17:19

Hinton requested a review from willmartian February 3, 2025 19:10

willmartian approved these changes Feb 3, 2025

View reviewed changes

Hinton removed the needs-qa Marks a PR as requiring QA approval label Feb 3, 2025

Hinton merged commit e5ffc16 into main Feb 3, 2025
86 of 87 checks passed

Hinton deleted the arch/control-flow branch February 3, 2025 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CL-553] Migrate CL to Control Flow syntax #12390

[CL-553] Migrate CL to Control Flow syntax #12390

Hinton commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024 •

edited

Loading

codecov bot commented Dec 13, 2024 •

edited

Loading

Hinton commented Jan 7, 2025

Hinton Jan 7, 2025

willmartian left a comment •

edited

Loading

willmartian Jan 10, 2025

Hinton Jan 13, 2025

willmartian commented Jan 10, 2025

willmartian commented Jan 10, 2025

Hinton commented Feb 3, 2025

[CL-553] Migrate CL to Control Flow syntax #12390

[CL-553] Migrate CL to Control Flow syntax #12390

Conversation

Hinton commented Dec 13, 2024 • edited Loading

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

github-actions bot commented Dec 13, 2024 • edited Loading

codecov bot commented Dec 13, 2024 • edited Loading

Codecov Report

Hinton commented Jan 7, 2025

Hinton Jan 7, 2025

Choose a reason for hiding this comment

willmartian left a comment • edited Loading

Choose a reason for hiding this comment

willmartian Jan 10, 2025

Choose a reason for hiding this comment

Hinton Jan 13, 2025

Choose a reason for hiding this comment

willmartian commented Jan 10, 2025

willmartian commented Jan 10, 2025

Hinton commented Feb 3, 2025

Hinton commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024 •

edited

Loading

codecov bot commented Dec 13, 2024 •

edited

Loading

willmartian left a comment •

edited

Loading