Skip to content

[PM-35224] Add route provider parameters support to featureFlaggedRoute#20235

Open
Banrion wants to merge 1 commit intomainfrom
dirt/pm-35224/featureflaggedroute-route-providers
Open

[PM-35224] Add route provider parameters support to featureFlaggedRoute#20235
Banrion wants to merge 1 commit intomainfrom
dirt/pm-35224/featureflaggedroute-route-providers

Conversation

@Banrion
Copy link
Copy Markdown
Contributor

@Banrion Banrion commented Apr 17, 2026

🎟️ Tracking

[Client] Add route provider parameters support to featureFlaggedRoute

📔 Objective

Extends featureFlaggedRoute to support scoped service providers on each side of a feature flag. Previously, any services exclusive to the flagged or default experience had to be registered at the module level and were instantiated unconditionally. Route-level providers create a child injector Angular only activates when the route is entered, enabling true lazy initialization.

Changes:

  • Adds optional flaggedRouteProviders and defaultRouteProviders to FeatureFlaggedRouteConfig
  • Adds a second @example showing scoped-provider usage
  • Fully backward compatible, existing callers are unaffected

@Banrion Banrion requested a review from a team as a code owner April 17, 2026 15:24
@Banrion Banrion requested a review from djsmith85 April 17, 2026 15:24
@sonarqubecloud
Copy link
Copy Markdown

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2026

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.12%. Comparing base (a4c457f) to head (26f4a73).
⚠️ Report is 24 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ngular/src/platform/utils/feature-flagged-route.ts 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #20235      +/-   ##
==========================================
+ Coverage   47.01%   47.12%   +0.11%     
==========================================
  Files        3892     3898       +6     
  Lines      117254   117586     +332     
  Branches    17933    18005      +72     
==========================================
+ Hits        55122    55414     +292     
- Misses      59645    59673      +28     
- Partials     2487     2499      +12     

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown
Contributor

Logo
Checkmarx One – Scan Summary & Details66664f76-1b2e-43f5-88e9-2275feefe635

Great job! No new security vulnerabilities introduced in this pull request

* // Only instantiated when the feature flag is OFF
* safeProvider({ provide: LegacyGroupsService, useClass: LegacyGroupsService, deps: [...] }),
* ],
* flaggedRouteProviders: [
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ What's the specific use case for this? Couldn't the existing and flagged component just create their own providers? As for example:
https://github.com/bitwarden/clients/blob/main/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.ts#L59

Then using this or the https://github.com/bitwarden/clients/blob/main/libs/angular/src/platform/guard/feature-flag.guard.ts would be sufficient.

I still wonder why we have two of these 🤷

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An example is in this pull request for Access Intelligence refactoring in the routing module.

Two main reason: performance and unintended background execution

Sever Access Intelligence services have been refactored that start observable pipelines in the constructor. Route level providers scope the instantiation to when that route is actually activated, so only when the feature flag is on. Many of these services are also needed in many components rather than just one. The main service that benefits from this approach is the AccessIntelligenceDataService replacing the RiskInsightsOrchestratorService.

We could work around this by changing the services by exposing an initialize method to start the service, but I feel this adds a lot of overhead. The proposed change felt clean and clear to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants