-
Notifications
You must be signed in to change notification settings - Fork 1.5k
PM-25525 debt fix system service provider dependency injection #16730
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PM-25525 debt fix system service provider dependency injection #16730
Conversation
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16730 +/- ##
==========================================
- Coverage 38.79% 38.78% -0.02%
==========================================
Files 3411 3410 -1
Lines 96907 96953 +46
Branches 14559 14560 +1
==========================================
+ Hits 37596 37603 +7
- Misses 57659 57700 +41
+ Partials 1652 1650 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
• remove unused logging methods in import.service.spec.ts • restore (unused) feature flag support in generator-services.module.ts • avoid type casting • clarify doc comments
• PlatformUtilsService.copyToClipboard() • PlatformUtilsService.readFromClipboard() • DEPRECATED PlatformUtilsService.showToast() these methods weren't being used, but in order to avoid breaking anything, I've added them
|
Removing review request as PR is marked as draft, let me know when it's ready! |
…T-Fix-SystemServiceProvider-dependency-injection pull latest from main to troubleshoot github action error
|
Changes in this PR impact the Autofill experience of the browser clientBIT has tested the core experience with these changes and all feature flags disabled. ✅ Fortunately, these BIT tests have passed! 🎉 |
Changes in this PR impact the Autofill experience of the browser clientBIT has tested the core experience with these changes and the feature flag configuration used by ✅ Fortunately, these BIT tests have passed! 🎉 |
|
This effort is being abandoned as it was not completed before the existing |




🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-25525?atlOrigin=eyJpIjoiNGUwOThmZjM4Y2QyNGExNWI1ODdjYzg5ZDE3ODYzNzAiLCJwIjoiaiJ9
📔 Objective
The system service provider aggregates several services into a single injectable type. This type isn’t compatible with Angular injection, has objects like the extension registry that aren’t properly configurable, and introduces multiple instances of itself.
Revise the system service provider to fix these issues.
Move the LogProvider and SemanticLogger to @bitwarden/logging
Preserve the observable in BitwardenClient
Write a facade over ConfigService and PlatformUtilsService that consolidates environmental awareness.
Consider introducing a “capabilities” abstraction for this.
The extension system provides an example of how the APIs may evolve.
The algorithm metadata provides an example of how capabilities can be surfaced.
Replace the SystemServiceProvider type with something that better composes with DI.
⏰ Reminders before review
🦮 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