-
Notifications
You must be signed in to change notification settings - Fork 452
chore: add decorators/annotations for DI of inversify #13043
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
Conversation
📝 Walkthrough""" WalkthroughThis change introduces widespread adoption of dependency injection throughout the codebase using the InversifyJS library. The update adds Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/main/src/plugin/authentication.tsOops! Something went wrong! :( ESLint: 9.30.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'svelte-eslint-parser' imported from /eslint.config.mjs packages/main/src/plugin/appearance-init.tsOops! Something went wrong! :( ESLint: 9.30.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'svelte-eslint-parser' imported from /eslint.config.mjs packages/api/src/configuration/models.tsOops! Something went wrong! :( ESLint: 9.30.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'svelte-eslint-parser' imported from /eslint.config.mjs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (69)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (66)
⏰ Context from checks skipped due to timeout of 90000ms (11)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/main/src/plugin/contribution-manager.ts (1)
77-101
: EnsureExec
is injectable and all dependencies are boundI couldn’t find an
@injectable()
decorator on yourExec
class or any explicitcontainer.bind(…)
calls for these types. Without these, Inversify won’t be able to resolve the injections in ContributionManager.• packages/main/src/plugin/util/exec.ts
Add at the top of the class:@injectable() export class Exec { … }• Your IoC container setup (e.g. in packages/main/src/plugin/index.ts or wherever you configure Inversify) needs to register each dependency:
container .bind<ApiSenderType>(ApiSenderType).toDynamicValue(buildApiSender).inSingletonScope() .bind(Directories).toSelf().inSingletonScope() .bind(ContainerProviderRegistry).toSelf().inSingletonScope() .bind(Exec).toSelf().inSingletonScope();Once those are in place, ContributionManager’s constructor injections will resolve correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (65)
packages/api/src/configuration/models.ts
(1 hunks)packages/main/src/plugin/api.ts
(1 hunks)packages/main/src/plugin/appearance-init.ts
(1 hunks)packages/main/src/plugin/authentication.ts
(3 hunks)packages/main/src/plugin/autostart-engine.ts
(1 hunks)packages/main/src/plugin/cancellation-token-registry.ts
(2 hunks)packages/main/src/plugin/certificates.ts
(3 hunks)packages/main/src/plugin/cli-tool-registry.ts
(2 hunks)packages/main/src/plugin/close-behavior.ts
(1 hunks)packages/main/src/plugin/color-registry.ts
(1 hunks)packages/main/src/plugin/command-registry.ts
(3 hunks)packages/main/src/plugin/commands-init.ts
(1 hunks)packages/main/src/plugin/configuration-registry.ts
(3 hunks)packages/main/src/plugin/confirmation-init.ts
(2 hunks)packages/main/src/plugin/container-registry.ts
(5 hunks)packages/main/src/plugin/context/context.ts
(2 hunks)packages/main/src/plugin/contribution-manager.ts
(3 hunks)packages/main/src/plugin/custompick/custompick-registry.ts
(2 hunks)packages/main/src/plugin/dialog-registry.ts
(2 hunks)packages/main/src/plugin/directories.ts
(2 hunks)packages/main/src/plugin/docker/docker-compatibility.ts
(3 hunks)packages/main/src/plugin/editor-init.ts
(1 hunks)packages/main/src/plugin/extension/catalog/extensions-catalog.ts
(2 hunks)packages/main/src/plugin/extension/extension-analyzer.ts
(2 hunks)packages/main/src/plugin/extension/extension-development-folders.ts
(2 hunks)packages/main/src/plugin/extension/extension-loader.ts
(4 hunks)packages/main/src/plugin/extension/extension-watcher.ts
(2 hunks)packages/main/src/plugin/featured/featured.ts
(1 hunks)packages/main/src/plugin/feedback-handler.ts
(2 hunks)packages/main/src/plugin/filesystem-monitoring.ts
(2 hunks)packages/main/src/plugin/icon-registry.ts
(2 hunks)packages/main/src/plugin/image-checker.ts
(2 hunks)packages/main/src/plugin/image-files-registry.ts
(2 hunks)packages/main/src/plugin/image-registry.ts
(4 hunks)packages/main/src/plugin/input-quickpick/input-quickpick-registry.ts
(2 hunks)packages/main/src/plugin/kubernetes/kube-generator-registry.ts
(3 hunks)packages/main/src/plugin/kubernetes/kubernetes-client.ts
(4 hunks)packages/main/src/plugin/learning-center-init.ts
(1 hunks)packages/main/src/plugin/libpod-api-enable/libpod-api-init.ts
(1 hunks)packages/main/src/plugin/menu-registry.ts
(2 hunks)packages/main/src/plugin/message-box.ts
(1 hunks)packages/main/src/plugin/navigation-items-init.ts
(1 hunks)packages/main/src/plugin/navigation/navigation-manager.ts
(1 hunks)packages/main/src/plugin/onboarding-registry.ts
(1 hunks)packages/main/src/plugin/open-devtools-init.ts
(1 hunks)packages/main/src/plugin/provider-registry.ts
(4 hunks)packages/main/src/plugin/proxy.ts
(4 hunks)packages/main/src/plugin/recommendations/recommendations-registry.ts
(1 hunks)packages/main/src/plugin/release-notes-banner-init.ts
(2 hunks)packages/main/src/plugin/safe-storage/safe-storage-registry.ts
(3 hunks)packages/main/src/plugin/statusbar/pin-registry.ts
(2 hunks)packages/main/src/plugin/statusbar/statusbar-providers-init.ts
(1 hunks)packages/main/src/plugin/statusbar/statusbar-registry.ts
(1 hunks)packages/main/src/plugin/tasks/notification-registry.ts
(2 hunks)packages/main/src/plugin/tasks/progress-impl.ts
(3 hunks)packages/main/src/plugin/tasks/task-manager.ts
(2 hunks)packages/main/src/plugin/telemetry/telemetry.ts
(3 hunks)packages/main/src/plugin/terminal-init.ts
(1 hunks)packages/main/src/plugin/tray-icon-color.ts
(1 hunks)packages/main/src/plugin/tray-menu-registry.ts
(2 hunks)packages/main/src/plugin/troubleshooting.ts
(2 hunks)packages/main/src/plugin/updater.ts
(2 hunks)packages/main/src/plugin/view-registry.ts
(2 hunks)packages/main/src/plugin/webview/webview-registry.ts
(3 hunks)packages/main/src/plugin/welcome/welcome-init.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (14)
packages/main/src/plugin/extension/extension-analyzer.ts (3)
packages/main/src/plugin/extension/extension-development-folders.ts (1)
injectable
(33-194)packages/main/src/plugin/extension/extension-watcher.ts (1)
injectable
(33-136)packages/main/src/plugin/extension/extension-loader.ts (1)
injectable
(117-1851)
packages/main/src/plugin/appearance-init.ts (7)
packages/main/src/plugin/confirmation-init.ts (1)
injectable
(23-43)packages/main/src/plugin/configuration-registry.ts (1)
injectable
(43-332)packages/main/src/plugin/editor-init.ts (1)
injectable
(25-47)packages/main/src/plugin/learning-center-init.ts (1)
injectable
(23-43)packages/main/src/plugin/navigation-items-init.ts (1)
injectable
(23-44)packages/main/src/plugin/release-notes-banner-init.ts (1)
injectable
(23-43)packages/api/src/configuration/models.ts (2)
IConfigurationRegistry
(101-101)IConfigurationRegistry
(102-116)
packages/main/src/plugin/icon-registry.ts (1)
packages/main/src/plugin/api.ts (2)
ApiSenderType
(21-21)ApiSenderType
(22-25)
packages/main/src/plugin/navigation-items-init.ts (8)
packages/main/src/plugin/close-behavior.ts (1)
injectable
(25-46)packages/main/src/plugin/confirmation-init.ts (1)
injectable
(23-43)packages/main/src/plugin/editor-init.ts (1)
injectable
(25-47)packages/main/src/plugin/learning-center-init.ts (1)
injectable
(23-43)packages/main/src/plugin/libpod-api-enable/libpod-api-init.ts (1)
injectable
(25-46)packages/main/src/plugin/open-devtools-init.ts (1)
injectable
(23-45)packages/main/src/plugin/release-notes-banner-init.ts (1)
injectable
(23-43)packages/api/src/configuration/models.ts (2)
IConfigurationRegistry
(101-101)IConfigurationRegistry
(102-116)
packages/main/src/plugin/editor-init.ts (8)
packages/main/src/plugin/confirmation-init.ts (1)
injectable
(23-43)packages/main/src/plugin/configuration-registry.ts (1)
injectable
(43-332)packages/main/src/plugin/learning-center-init.ts (1)
injectable
(23-43)packages/main/src/plugin/libpod-api-enable/libpod-api-init.ts (1)
injectable
(25-46)packages/main/src/plugin/open-devtools-init.ts (1)
injectable
(23-45)packages/main/src/plugin/navigation-items-init.ts (1)
injectable
(23-44)packages/main/src/plugin/release-notes-banner-init.ts (1)
injectable
(23-43)packages/api/src/configuration/models.ts (2)
IConfigurationRegistry
(101-101)IConfigurationRegistry
(102-116)
packages/main/src/plugin/extension/catalog/extensions-catalog.ts (3)
packages/main/src/plugin/proxy.ts (1)
proxy
(177-179)packages/api/src/configuration/models.ts (2)
IConfigurationRegistry
(101-101)IConfigurationRegistry
(102-116)packages/main/src/plugin/api.ts (2)
ApiSenderType
(21-21)ApiSenderType
(22-25)
packages/main/src/plugin/tray-icon-color.ts (1)
packages/api/src/configuration/models.ts (2)
IConfigurationRegistry
(101-101)IConfigurationRegistry
(102-116)
packages/main/src/plugin/release-notes-banner-init.ts (6)
packages/main/src/plugin/confirmation-init.ts (1)
injectable
(23-43)packages/main/src/plugin/configuration-registry.ts (1)
injectable
(43-332)packages/main/src/plugin/editor-init.ts (1)
injectable
(25-47)packages/main/src/plugin/learning-center-init.ts (1)
injectable
(23-43)packages/main/src/plugin/navigation-items-init.ts (1)
injectable
(23-44)packages/api/src/configuration/models.ts (2)
IConfigurationRegistry
(101-101)IConfigurationRegistry
(102-116)
packages/main/src/plugin/extension/extension-development-folders.ts (3)
packages/main/src/plugin/extension/extension-analyzer.ts (1)
injectable
(59-134)packages/api/src/configuration/models.ts (2)
IConfigurationRegistry
(101-101)IConfigurationRegistry
(102-116)packages/main/src/plugin/api.ts (2)
ApiSenderType
(21-21)ApiSenderType
(22-25)
packages/main/src/plugin/docker/docker-compatibility.ts (1)
packages/api/src/configuration/models.ts (2)
IConfigurationRegistry
(101-101)IConfigurationRegistry
(102-116)
packages/main/src/plugin/tasks/notification-registry.ts (2)
packages/main/src/plugin/tasks/task-manager.ts (1)
injectable
(34-254)packages/main/src/plugin/api.ts (2)
ApiSenderType
(21-21)ApiSenderType
(22-25)
packages/main/src/plugin/extension/extension-loader.ts (6)
packages/main/src/plugin/extension/extension-analyzer.ts (1)
injectable
(59-134)packages/main/src/plugin/extension/extension-development-folders.ts (1)
injectable
(33-194)packages/api/src/configuration/models.ts (2)
IConfigurationRegistry
(101-101)IConfigurationRegistry
(102-116)packages/main/src/plugin/api.ts (2)
ApiSenderType
(21-21)ApiSenderType
(22-25)packages/main/src/plugin/proxy.ts (1)
proxy
(177-179)packages/main/src/plugin/util/exec.ts (2)
Exec
(46-242)exec
(49-241)
packages/main/src/plugin/image-registry.ts (3)
packages/main/src/plugin/certificates.ts (1)
injectable
(34-163)packages/main/src/plugin/telemetry/telemetry.ts (1)
injectable
(65-441)packages/main/src/plugin/api.ts (2)
ApiSenderType
(21-21)ApiSenderType
(22-25)
packages/main/src/plugin/color-registry.ts (2)
packages/main/src/plugin/configuration-registry.ts (1)
injectable
(43-332)packages/main/src/plugin/api.ts (2)
ApiSenderType
(21-21)ApiSenderType
(22-25)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: smoke e2e tests (development)
- GitHub Check: smoke e2e tests (production)
- GitHub Check: unit tests / macos-15
- GitHub Check: Linux
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: unit tests / windows-2022
- GitHub Check: Windows
- GitHub Check: typecheck
- GitHub Check: macOS
- GitHub Check: linter, formatters
🔇 Additional comments (182)
packages/main/src/plugin/troubleshooting.ts (2)
23-23
: LGTM: Import added for dependency injectionThe
inversify
import is correctly added to support the@injectable()
decorator.
35-35
: LGTM: Injectable decorator properly appliedThe
@injectable()
decorator correctly enables theTroubleshooting
class for dependency injection management. Since this class has no dependencies, no constructor injection is needed.packages/main/src/plugin/view-registry.ts (3)
2-2
: LGTM: Copyright year updatedThe copyright year is appropriately updated to 2025.
18-18
: LGTM: Import added for dependency injectionThe
inversify
import is correctly added to support the@injectable()
decorator.
24-24
: LGTM: Injectable decorator properly appliedThe
@injectable()
decorator correctly enables theViewRegistry
class for dependency injection management.packages/api/src/configuration/models.ts (1)
101-101
: LGTM: Injection token properly definedThe
IConfigurationRegistry
symbol is correctly defined usingSymbol.for()
to create a globally accessible injection token. This follows InversifyJS best practices and enables proper dependency injection for the configuration registry interface.packages/main/src/plugin/extension/extension-analyzer.ts (2)
24-24
: LGTM: Import added for dependency injectionThe
inversify
import is correctly added to support the@injectable()
decorator.
59-59
: LGTM: Injectable decorator enables dependency injectionThe
@injectable()
decorator correctly enables theExtensionAnalyzer
class for dependency injection. This is essential since this class is injected into other components likeExtensionLoader
andExtensionDevelopmentFolders
as seen in the related code snippets.packages/main/src/plugin/api.ts (1)
21-21
: LGTM: Injection token properly definedThe
ApiSenderType
symbol is correctly defined usingSymbol.for()
to create a globally accessible injection token. This follows the established pattern where both a type and symbol share the same name, enabling both compile-time type checking and runtime dependency injection.packages/main/src/plugin/certificates.ts (1)
24-24
: LGTM! Correct implementation of Inversify DI annotations.The
@injectable()
decorator and import are properly added to enable dependency injection for theCertificates
class without any functional changes.Also applies to: 34-34
packages/main/src/plugin/filesystem-monitoring.ts (1)
24-24
: LGTM! Proper DI annotation for FilesystemMonitoring.The
@injectable()
decorator is correctly applied to theFilesystemMonitoring
class, enabling it for dependency injection while leaving the implementation class unchanged.Also applies to: 110-110
packages/main/src/plugin/directories.ts (1)
23-23
: LGTM! Correct DI implementation for Directories class.The
@injectable()
decorator is properly added to enable dependency injection for theDirectories
class without modifying its functionality.Also applies to: 26-26
packages/main/src/plugin/cancellation-token-registry.ts (1)
19-19
: LGTM! Proper DI setup for CancellationTokenRegistry.The
@injectable()
decorator and import are correctly implemented to enable dependency injection for theCancellationTokenRegistry
class.Also applies to: 23-23
packages/main/src/plugin/kubernetes/kube-generator-registry.ts (1)
18-18
: LGTM! Consistent DI implementation for KubeGeneratorRegistry.The
@injectable()
decorator is properly applied with the correct import, enabling dependency injection for theKubeGeneratorRegistry
class while preserving its functionality.Also applies to: 44-44
packages/main/src/plugin/input-quickpick/input-quickpick-registry.ts (4)
25-25
: LGTM: Clean dependency injection integration.The inversify imports are properly added to support the DI framework integration.
27-27
: LGTM: Correct import conversion for injection token.The import is correctly changed from type-only to value import to support the ApiSenderType injection token.
30-30
: LGTM: Proper class decoration for DI.The @Injectable() decorator is correctly applied to enable the class for dependency injection.
50-50
: LGTM: Correct constructor parameter injection.The constructor parameter is properly decorated with @Inject(ApiSenderType) for dependency injection while maintaining the same functionality.
packages/main/src/plugin/custompick/custompick-registry.ts (3)
2-2
: LGTM: Copyright year update.The copyright year is correctly updated to reflect 2025.
19-19
: LGTM: Proper DI imports and injection token.The inversify imports and ApiSenderType value import are correctly added to support dependency injection.
Also applies to: 21-21
25-25
: LGTM: Consistent DI implementation.The class is properly decorated with @Injectable() and the constructor parameter is correctly decorated with @Inject(ApiSenderType), following the same pattern as other classes in the codebase.
Also applies to: 30-30
packages/main/src/plugin/cli-tool-registry.ts (3)
2-2
: LGTM: Copyright year update.The copyright year is appropriately updated to 2025.
28-28
: LGTM: Proper DI setup.The inversify imports and ApiSenderType value import are correctly added to support the dependency injection framework.
Also applies to: 33-33
38-38
: LGTM: Consistent DI pattern implementation.The @Injectable() decorator and @Inject(ApiSenderType) constructor parameter annotation follow the established pattern for dependency injection integration across the codebase.
Also applies to: 40-40
packages/main/src/plugin/welcome/welcome-init.ts (2)
19-19
: LGTM: Proper DI imports for configuration registry.The inversify imports and IConfigurationRegistry imports are correctly added to support dependency injection with the configuration registry.
Also applies to: 21-21
25-25
: LGTM: Consistent DI pattern with configuration injection.The @Injectable() decorator and @Inject(IConfigurationRegistry) constructor parameter annotation properly implement dependency injection for the configuration registry dependency.
Also applies to: 27-27
packages/main/src/plugin/terminal-init.ts (2)
19-19
: LGTM: Correct DI imports for configuration management.The inversify imports and IConfigurationRegistry imports are properly added to enable dependency injection for configuration management.
Also applies to: 21-21
25-25
: LGTM: Proper DI implementation for initialization class.The @Injectable() decorator and @Inject(IConfigurationRegistry) constructor parameter annotation correctly implement the dependency injection pattern consistent with other initialization classes.
Also applies to: 29-29
packages/main/src/plugin/icon-registry.ts (4)
21-21
: LGTM! Proper Inversify import added.The import statement correctly adds the necessary decorators for dependency injection.
28-28
: LGTM! Correct import conversion for injection token.Converting the
ApiSenderType
import from type-only to value import is necessary for runtime injection.
30-30
: LGTM! Injectable decorator properly applied.The
@injectable()
decorator correctly marks the class for dependency injection.
35-35
: LGTM! Injection decorator correctly applied.The
@inject(ApiSenderType)
decorator properly specifies the injection token for the constructor parameter.packages/main/src/plugin/context/context.ts (3)
24-26
: LGTM! Consistent DI implementation.The import changes and decorator usage follow the same pattern as other files in this PR, ensuring consistency across the codebase.
29-29
: LGTM! Injectable decorator properly applied.The
@injectable()
decorator correctly enables the Context class for dependency injection.
33-33
: LGTM! Proper injection configuration.The constructor parameter is correctly configured with the
@inject(ApiSenderType)
decorator.packages/main/src/plugin/message-box.ts (4)
18-18
: LGTM! Consistent Inversify import pattern.The import statement follows the established pattern across the codebase.
22-22
: LGTM! Correct import conversion for injection.Converting the
ApiSenderType
import from type-only to value import enables runtime injection.
27-27
: LGTM! Injectable decorator properly applied.The
@injectable()
decorator correctly marks the MessageBox class for dependency injection.
33-33
: LGTM! Proper injection configuration.The constructor parameter is correctly configured with the
@inject(ApiSenderType)
decorator.packages/main/src/plugin/close-behavior.ts (4)
19-19
: LGTM! Consistent Inversify import.The import statement follows the established pattern for adding DI support.
21-21
: LGTM! Correct import conversion for injection token.Converting the
IConfigurationRegistry
import from type-only to value import enables runtime injection of the configuration registry.
25-25
: LGTM! Injectable decorator properly applied.The
@injectable()
decorator correctly marks the CloseBehavior class for dependency injection.
27-27
: LGTM! Appropriate injection token used.The
@inject(IConfigurationRegistry)
decorator correctly specifies the injection token for the configuration registry dependency.packages/main/src/plugin/statusbar/statusbar-registry.ts (4)
19-19
: LGTM! Consistent Inversify import pattern.The import statement maintains consistency with the DI implementation across all files.
23-23
: LGTM! Correct import conversion for injection.Converting the
ApiSenderType
import from type-only to value import enables runtime injection.
26-26
: LGTM! Injectable decorator properly applied.The
@injectable()
decorator correctly marks the StatusBarRegistry class for dependency injection.
30-30
: LGTM! Proper injection configuration.The constructor parameter is correctly configured with the
@inject(ApiSenderType)
decorator, completing the DI setup.packages/main/src/plugin/safe-storage/safe-storage-registry.ts (1)
24-26
: LGTM! Inversify integration implemented correctly.The dependency injection setup follows proper Inversify patterns:
- Class properly decorated with
@injectable()
- Constructor parameter correctly injected with
@inject(Directories)
- Import correctly changed from type-only to value import for runtime injection
Also applies to: 35-43
packages/main/src/plugin/webview/webview-registry.ts (1)
26-28
: LGTM! Proper Inversify integration with correct injection token usage.The dependency injection implementation is correct:
@injectable()
decorator properly applied to the class@inject(ApiSenderType)
correctly injects the API sender dependency- Import statements appropriately updated to support runtime injection
Also applies to: 80-96
packages/main/src/plugin/extension/extension-watcher.ts (1)
21-26
: LGTM! Clean Inversify integration for extension monitoring.The dependency injection implementation is properly executed:
@injectable()
decorator correctly applied@inject(FilesystemMonitoring)
properly injects the file system monitoring dependency- Import statements correctly updated from type-only to value imports
Also applies to: 33-44
packages/main/src/plugin/tasks/progress-impl.ts (1)
19-27
: LGTM! Comprehensive Inversify integration with multiple dependencies.The dependency injection setup is correctly implemented:
@injectable()
decorator properly applied to the class- All three constructor parameters correctly injected with appropriate
@inject()
decorators- Import statements properly updated from type-only to value imports to support runtime injection
- Multiple dependency injection handled cleanly
Also applies to: 41-50
packages/main/src/plugin/dialog-registry.ts (1)
22-25
: LGTM! Proper Inversify integration with generic type injection.The dependency injection implementation is correct:
@injectable()
decorator properly applied@inject(Deferred<BrowserWindow>)
correctly handles generic type injection- Import statements appropriately updated to support runtime injection
- Generic type usage in injection token is handled properly
Also applies to: 30-38
packages/main/src/plugin/release-notes-banner-init.ts (4)
2-2
: LGTM: Copyright year updated appropriately.The copyright year has been updated to include 2025, which is consistent with the current timeframe.
19-19
: LGTM: Inversify decorators imported correctly.The
inject
andinjectable
decorators are properly imported from the Inversify library to support dependency injection.
21-21
: LGTM: Import converted from type-only to value import.The
IConfigurationRegistry
import is correctly changed from a type-only import to a value import to support runtime dependency injection.
23-25
: LGTM: Dependency injection implementation follows established pattern.The class is properly decorated with
@injectable()
and the constructor parameter is correctly annotated with@inject(IConfigurationRegistry)
. This follows the exact same pattern used across other initialization classes likeLearningCenterInit
,ConfirmationInit
, andEditorInit
as shown in the relevant code snippets.packages/main/src/plugin/extension/extension-loader.ts (5)
2-2
: LGTM: Copyright year updated appropriately.The copyright year has been updated to include 2025, which is consistent with the current timeframe.
25-25
: LGTM: Inversify decorators imported correctly.The
inject
andinjectable
decorators are properly imported from the Inversify library to support dependency injection.
27-92
: LGTM: Import statements properly converted for dependency injection.The imports have been correctly updated from type-only imports to value imports where needed to support runtime dependency injection. This includes removing
type
qualifiers from imports that need to be available at runtime for injection tokens.
117-117
: LGTM: Class properly decorated for dependency injection.The
ExtensionLoader
class is correctly decorated with@injectable()
to enable it to be managed by the Inversify container.
140-215
: LGTM: Constructor parameters properly configured for dependency injection.All constructor parameters are correctly annotated with
@inject()
decorators specifying their injection tokens. The implementation follows the established dependency injection pattern and uses the appropriate symbols likeCommandRegistry
,IConfigurationRegistry
,ApiSenderType
, etc. The injection tokens align with the symbols defined in the relevant code snippets.While the constructor has a large number of dependencies (30+ parameters), this reflects the existing architecture and responsibilities of the
ExtensionLoader
class rather than being a new issue introduced by the dependency injection changes.packages/main/src/plugin/command-registry.ts (4)
2-2
: LGTM: Copyright year updated appropriately.The copyright year has been updated to include 2025, which is consistent with the current timeframe.
19-19
: LGTM: Inversify decorators imported correctly.The
inject
andinjectable
decorators are properly imported from the Inversify library to support dependency injection.
21-24
: LGTM: Import statements properly organized for dependency injection.The imports are correctly organized with type-only imports separated from value imports. The
ApiSenderType
andTelemetry
imports are converted to value imports to support runtime dependency injection.
44-54
: LGTM: Dependency injection implementation is correct.The
CommandRegistry
class is properly decorated with@injectable()
and both constructor parameters are correctly annotated with their respective injection tokens:@inject(ApiSenderType)
and@inject(Telemetry)
. This follows the established dependency injection pattern used throughout the codebase.packages/main/src/plugin/appearance-init.ts (3)
20-20
: LGTM: InversifyJS imports added correctly.The import statement properly includes both
inject
andinjectable
decorators needed for dependency injection.
22-22
: LGTM: Import updated to support runtime injection.The import correctly changes
IConfigurationRegistry
from a type-only import to a value import, enabling it to be used as an injection token at runtime.
28-30
: LGTM: Dependency injection decorators applied correctly.The class is properly decorated with
@injectable()
and the constructor parameter uses the correct@inject(IConfigurationRegistry)
pattern, consistent with other configuration-related classes in the codebase.packages/main/src/plugin/statusbar/pin-registry.ts (3)
19-19
: LGTM: InversifyJS imports added correctly.The import statement properly includes both decorators needed for dependency injection.
21-25
: LGTM: All dependency imports updated for runtime injection.All required dependencies are correctly changed from type-only imports to value imports, enabling them to be used as injection tokens at runtime.
39-39
: LGTM: Comprehensive dependency injection implementation.The class is properly decorated with
@injectable()
and all constructor parameters are correctly annotated with their respective@inject()
decorators. This follows the established pattern and enables full dependency injection for thePinRegistry
class.Also applies to: 47-56
packages/main/src/plugin/menu-registry.ts (3)
18-18
: LGTM: InversifyJS imports added correctly.The import statement properly includes both decorators needed for dependency injection.
22-22
: LGTM: CommandRegistry import updated for runtime injection.The import correctly changes from type-only to value import, enabling
CommandRegistry
to be used as an injection token at runtime.
25-25
: LGTM: Dependency injection decorators applied correctly.The class is properly decorated with
@injectable()
and the constructor parameter uses the correct@inject(CommandRegistry)
pattern, enabling dependency injection for theMenuRegistry
class.Also applies to: 29-29
packages/main/src/plugin/commands-init.ts (3)
21-21
: LGTM: InversifyJS imports added correctly.The import statement properly includes both decorators needed for dependency injection.
28-32
: LGTM: All dependency imports updated for runtime injection.All required dependencies (
ApiSenderType
,CommandRegistry
,ContainerProviderRegistry
,NavigationManager
,TaskManager
) are correctly changed from type-only imports to value imports, enabling them to be used as injection tokens at runtime.
35-35
: LGTM: Comprehensive dependency injection implementation.The class is properly decorated with
@injectable()
and all constructor parameters are correctly annotated with their respective@inject()
decorators. This follows the established pattern and enables full dependency injection for theCommandsInit
class.Also applies to: 39-48
packages/main/src/plugin/tray-icon-color.ts (3)
19-19
: LGTM: InversifyJS imports added correctly.The import statement properly includes both decorators needed for dependency injection.
21-21
: LGTM: Import correctly updated for mixed usage.The import properly handles both type-only (
IConfigurationNode
) and value (IConfigurationRegistry
) imports, enabling runtime injection of the registry while keeping the node interface as a type.
23-25
: LGTM: Dependency injection decorators applied correctly.The class is properly decorated with
@injectable()
and the constructor parameter uses the correct@inject(IConfigurationRegistry)
pattern, consistent with other configuration-related classes in the codebase such asAppearanceInit
.packages/main/src/plugin/learning-center-init.ts (3)
19-19
: LGTM: Proper InversifyJS imports added.The import of
inject
andinjectable
decorators from inversify is correct and necessary for dependency injection.
21-21
: LGTM: Correct import change for runtime injection.Changing from type-only import to value import for
IConfigurationRegistry
is necessary since the@inject
decorator needs the actual symbol at runtime.
23-25
: Verification needed: Confirm IConfigurationRegistry bindingI’ve verified that
IConfigurationRegistry
is defined inpackages/api/src/configuration/models.ts
asSymbol.for('IConfigurationRegistry')
- There’s no binding for this token in the plugin’s own container modules (which is expected)
Please confirm that the host application’s main Inversify container (e.g. in Theia core) binds
IConfigurationRegistry
to its default implementation before plugins are loaded. If that global binding exists, no changes are needed here.packages/main/src/plugin/confirmation-init.ts (1)
19-25
: LGTM: Consistent dependency injection implementation.The changes follow the same correct pattern as other files in this PR:
- Proper InversifyJS imports
- Correct
@injectable()
decorator on the class- Appropriate
@inject(IConfigurationRegistry)
on constructor parameter- Necessary import change from type-only to value import
This consistency demonstrates a systematic and well-executed refactoring.
packages/main/src/plugin/image-checker.ts (2)
27-27
: LGTM: Proper InversifyJS imports added.The import of
inject
andinjectable
decorators is correct and consistent with the dependency injection pattern.
31-31
: LGTM: Correct import change for ApiSenderType.Changing from type-only import to value import for
ApiSenderType
is necessary for the@inject
decorator to function at runtime.packages/main/src/plugin/configuration-registry.ts (3)
23-23
: LGTM: Proper InversifyJS imports added.The import of
inject
andinjectable
decorators is correct and necessary for dependency injection.
37-39
: LGTM: Correct import changes for multiple dependencies.Changing from type-only imports to value imports for
ApiSenderType
andDirectories
is necessary since the@inject
decorators need the actual symbols at runtime.
43-66
: Verify DI Container BindingsI couldn’t find any
bind(...)
calls forApiSenderType
orDirectories
in your container configuration. Please ensure both are registered in your Inversify container (e.g. in yourcontainer-registry.ts
or equivalent), for example:// packages/main/src/plugin/container-registry.ts container.bind<ApiSenderType>(ApiSenderType) .toConstantValue(buildApiSender()); container.bind<Directories>(Directories) .to(Directories) .inSingletonScope();• Check that
ApiSenderType
is bound to the actual sender (e.g.buildApiSender()
)
• Check thatDirectories
is bound (singleton or appropriate scope)
• Verify container initialization is loaded beforeConfigurationRegistry
is constructedpackages/main/src/plugin/contribution-manager.ts (2)
23-23
: LGTM: Proper InversifyJS imports added.The import of
inject
andinjectable
decorators is correct and consistent with the dependency injection pattern.
29-32
: LGTM: Correct import changes for multiple dependencies.Changing from type-only imports to value imports for
ApiSenderType
,ContainerProviderRegistry
,Directories
, andExec
is necessary since the@inject
decorators need the actual symbols/classes at runtime.packages/main/src/plugin/featured/featured.ts (4)
19-19
: LGTM: Inversify imports added correctly.The import statement for InversifyJS decorators is properly added to support dependency injection.
21-23
: LGTM: Import changes to support runtime injection.The imports have been correctly changed from type-only to value imports to enable runtime dependency injection while preserving type information for the API import.
31-31
: LGTM: Injectable decorator applied correctly.The
@injectable()
decorator is properly applied to theFeatured
class to enable it for dependency injection container management.
35-40
: LGTM: Constructor injection implemented correctly.The constructor parameters are properly decorated with
@inject()
decorators using the appropriate class constructors as injection tokens. This follows InversifyJS best practices.packages/main/src/plugin/authentication.ts (4)
30-30
: LGTM: Inversify imports added correctly.The import statement for InversifyJS decorators is properly added to support dependency injection.
32-34
: LGTM: Import changes to support runtime injection.The imports have been correctly changed from type-only to value imports for
ApiSenderType
andMessageBox
to enable runtime dependency injection.
99-99
: LGTM: Injectable decorator applied correctly.The
@injectable()
decorator is properly applied to theAuthenticationImpl
class to enable dependency injection.
136-141
: LGTM: Constructor injection implemented correctly.The constructor parameters are properly decorated with
@inject()
decorators using the appropriate injection tokens (ApiSenderType
symbol andMessageBox
class). This follows InversifyJS best practices for dependency injection.packages/main/src/plugin/navigation-items-init.ts (3)
19-19
: LGTM: Inversify imports added correctly.The import statement for InversifyJS decorators is properly added to support dependency injection.
21-21
: LGTM: Import style updated for dependency injection.The mixed import style with
type
qualifier forIConfigurationNode
and value import forIConfigurationRegistry
is correct and consistent with the pattern used in other initialization classes throughout the codebase.
23-25
: LGTM: Dependency injection implemented correctly.The
@injectable()
decorator and@inject(IConfigurationRegistry)
constructor parameter decoration follow the established pattern seen in other initialization classes likeLearningCenterInit
,ConfirmationInit
, andEditorInit
.packages/main/src/plugin/statusbar/statusbar-providers-init.ts (3)
19-19
: LGTM: Inversify imports added correctly.The import statement for InversifyJS decorators is properly added to support dependency injection.
21-21
: LGTM: Import style updated for dependency injection.The mixed import style with
type
qualifier forIConfigurationNode
and value import forIConfigurationRegistry
is correct and consistent with other initialization classes.
23-25
: LGTM: Dependency injection implemented correctly.The
@injectable()
decorator and@inject(IConfigurationRegistry)
constructor parameter decoration follow the established pattern for initialization classes throughout the codebase.packages/main/src/plugin/image-files-registry.ts (4)
26-26
: LGTM: Inversify imports added correctly.The import statement for InversifyJS decorators is properly added to support dependency injection.
28-33
: LGTM: Import changes to support runtime injection.The imports have been correctly updated to support dependency injection:
- Mixed import style for configuration types preserves type safety
ApiSenderType
andContext
changed to value imports for runtime injection
43-43
: LGTM: Injectable decorator applied correctly.The
@injectable()
decorator is properly applied to theImageFilesRegistry
class to enable dependency injection container management.
50-57
: LGTM: Multiple constructor dependencies injected correctly.The constructor parameters are properly decorated with
@inject()
decorators using the appropriate injection tokens:
ApiSenderType
symbol for the API senderIConfigurationRegistry
symbol for configuration registryContext
class for the context serviceThis follows InversifyJS best practices for multi-dependency injection.
packages/main/src/plugin/libpod-api-enable/libpod-api-init.ts (2)
19-21
: LGTM! Correct InversifyJS import setup.The import changes properly support dependency injection by importing the necessary decorators and using mixed imports for the configuration types - type-only for
IConfigurationNode
and value import forIConfigurationRegistry
injection token.
25-27
: LGTM! Proper dependency injection decorators applied.The
@injectable()
decorator on the class and@inject(IConfigurationRegistry)
on the constructor parameter correctly enable InversifyJS dependency injection while maintaining the existing functionality.packages/main/src/plugin/autostart-engine.ts (2)
19-24
: LGTM! Correct import setup for dependency injection.The imports are properly updated to support InversifyJS decorators and runtime injection, converting
ProviderRegistry
from type-only to value import as needed for injection.
27-36
: LGTM! Proper dependency injection setup.The class is correctly decorated with
@injectable()
and both constructor parameters use appropriate@inject()
decorators -IConfigurationRegistry
symbol for the configuration registry andProviderRegistry
class for the provider registry.packages/main/src/plugin/editor-init.ts (2)
19-21
: LGTM! Consistent import pattern for DI support.The import setup correctly follows the established pattern with mixed imports - type-only for
IConfigurationNode
and value import for theIConfigurationRegistry
injection token.
25-27
: LGTM! Proper InversifyJS decorator usage.The
@injectable()
decorator and@inject(IConfigurationRegistry)
parameter decoration correctly enable dependency injection, consistent with other similar init classes in the codebase.packages/main/src/plugin/feedback-handler.ts (2)
20-22
: LGTM! Proper import setup for dependency injection.The InversifyJS decorators are imported and
ExtensionLoader
is correctly changed from type-only to value import to support runtime injection.
27-31
: LGTM! Correct dependency injection implementation.The
@injectable()
decorator on the class and@inject(ExtensionLoader)
on the constructor parameter properly enable dependency injection for the ExtensionLoader dependency.packages/main/src/plugin/telemetry/telemetry.ts (2)
33-36
: LGTM! Consistent import pattern for dependency injection.The InversifyJS decorators are properly imported and the configuration imports use the established mixed import pattern with type annotation for
IConfigurationNode
and value import for theIConfigurationRegistry
injection token.
65-94
: LGTM! Proper dependency injection setup for Telemetry class.The
@injectable()
decorator on the class and@inject(IConfigurationRegistry)
on the constructor parameter correctly enable InversifyJS dependency injection while preserving all existing functionality. The constructor formatting is clean and follows the established pattern.packages/main/src/plugin/extension/catalog/extensions-catalog.ts (4)
22-22
: LGTM: Inversify imports added correctly.The import statement for
inject
andinjectable
decorators is properly added to support dependency injection.
24-31
: LGTM: Import changes support runtime injection.The imports have been correctly changed from type-only imports to value imports, which is necessary for Inversify to inject these dependencies at runtime. The injection tokens (
ApiSenderType
,IConfigurationRegistry
) align with the symbols defined in the relevant code snippets.
38-38
: LGTM: Class properly marked as injectable.The
@injectable()
decorator correctly marks this class for dependency injection management by the Inversify container.
47-55
: LGTM: Constructor injection implemented correctly.The constructor parameters are properly decorated with
@inject()
specifying the appropriate injection tokens:
Certificates
andProxy
use class types as tokensIConfigurationRegistry
andApiSenderType
use symbol tokensThis follows Inversify best practices and maintains the existing constructor signature.
packages/main/src/plugin/open-devtools-init.ts (3)
19-19
: LGTM: Inversify imports added correctly.The import statement for dependency injection decorators is properly added.
21-21
: LGTM: Import change enables runtime injection.The
IConfigurationRegistry
import has been correctly changed from type-only to value import to support dependency injection.
23-25
: LGTM: Dependency injection properly implemented.The class is correctly marked as
@injectable()
and the constructor parameter uses the appropriate@inject(IConfigurationRegistry)
decorator with the symbol token defined in the configuration models.packages/main/src/plugin/tray-menu-registry.ts (4)
21-21
: LGTM: Inversify imports added correctly.The import statement for dependency injection decorators is properly added.
26-29
: LGTM: Dependencies imported for runtime injection.All dependency imports have been correctly changed from type-only to value imports to support Inversify dependency injection.
42-42
: LGTM: Class properly marked as injectable.The
@injectable()
decorator correctly enables this class for dependency injection management.
47-56
: LGTM: Multiple dependencies injected correctly.All four constructor parameters are properly decorated with
@inject()
using their respective class types as injection tokens. This follows Inversify best practices for constructor injection with multiple dependencies.packages/main/src/plugin/onboarding-registry.ts (3)
21-21
: LGTM: Inversify imports added correctly.The import statement for dependency injection decorators is properly added.
27-27
: LGTM: Context import updated for injection.The
Context
import has been correctly changed from type-only to value import to enable dependency injection.
30-34
: LGTM: Dependency injection correctly implemented.The class is properly marked as
@injectable()
and the constructor parameter uses the correct@inject(Context)
decorator with the class type as the injection token.packages/main/src/plugin/proxy.ts (5)
2-2
: LGTM: Copyright year updated appropriately.The copyright year range has been updated to include 2025, which is appropriate for this maintenance change.
20-20
: LGTM: Inversify imports added correctly.The import statement for dependency injection decorators is properly added.
23-24
: LGTM: Dependencies imported for runtime injection.Both
Certificates
andIConfigurationRegistry
imports have been correctly changed from type-only to value imports to support dependency injection.
58-58
: LGTM: Class properly marked as injectable.The
@injectable()
decorator correctly enables this class for dependency injection management by the Inversify container.
69-74
: LGTM: Constructor injection implemented correctly.Both constructor parameters are properly decorated with
@inject()
using their appropriate injection tokens:
IConfigurationRegistry
uses the symbol tokenCertificates
uses the class type as tokenThis mixed approach is valid in Inversify and follows the established patterns in the codebase.
packages/main/src/plugin/docker/docker-compatibility.ts (6)
2-2
: LGTM: Copyright year update.The copyright year has been appropriately updated to include 2025.
22-22
: LGTM: Inversify imports added correctly.The
inject
andinjectable
decorators are properly imported from the inversify package.
25-30
: LGTM: Import restructuring for dependency injection.The imports have been correctly restructured to:
- Import
IConfigurationRegistry
symbol for injection token- Separate type-only imports from runtime imports
- Maintain proper import organization
33-33
: LGTM: ProviderRegistry import change for runtime injection.The import has been correctly changed from a type-only import to a full import to enable runtime injection. This is necessary for the inversify container to resolve the dependency.
35-35
: LGTM: @Injectable() decorator added correctly.The
@injectable()
decorator has been properly applied to theDockerCompatibility
class, enabling it to be managed by the InversifyJS container.
47-50
: LGTM: Constructor parameter injection configured correctly.The constructor parameters have been properly annotated with
@inject()
decorators using the appropriate injection tokens:
IConfigurationRegistry
symbol for configuration registry injectionProviderRegistry
class for provider registry injectionThis follows InversifyJS best practices for dependency injection.
packages/main/src/plugin/container-registry.ts (4)
31-31
: LGTM: Inversify imports added correctly.The
inject
andinjectable
decorators are properly imported from the inversify package.
66-66
: LGTM: Import changes for runtime dependency injection.The imports have been correctly changed from type-only imports to full imports for the following dependencies:
ApiSenderType
from./api.js
ConfigurationRegistry
from./configuration-registry.js
ImageRegistry
from./image-registry.js
Telemetry
from./telemetry/telemetry.js
These changes are necessary to enable runtime injection by the InversifyJS container.
Also applies to: 68-68, 82-82, 84-84
111-111
: LGTM: @Injectable() decorator added correctly.The
@injectable()
decorator has been properly applied to theContainerProviderRegistry
class, enabling it to be managed by the InversifyJS container.
124-133
: LGTM: Constructor parameter injection configured correctly.The constructor parameters have been properly annotated with
@inject()
decorators using the appropriate injection tokens:
ApiSenderType
for the API senderConfigurationRegistry
class for configuration registryImageRegistry
class for image registryTelemetry
class for telemetry serviceThe injection configuration follows InversifyJS best practices and maintains clear separation of concerns.
packages/main/src/plugin/image-registry.ts (5)
2-2
: LGTM: Copyright year updateStandard maintenance update to reflect 2025.
32-32
: LGTM: InversifyJS imports addedCorrectly imports the necessary decorators for dependency injection.
39-43
: LGTM: Import changes for runtime injectionConverting from type-only imports to value imports is necessary to enable runtime dependency injection. The imported types (ApiSenderType, Certificates, Proxy, Telemetry) are correctly converted.
53-53
: LGTM: Injectable decorator addedThe 8000
@injectable()
decorator properly enables the ImageRegistry class to be managed by the Inversify container.
74-81
: LGTM: Constructor parameter injectionThe
@inject()
decorators are correctly applied with appropriate tokens:
ApiSenderType
for API communicationTelemetry
for telemetry serviceCertificates
for certificate managementProxy
for proxy settingsThe implementation follows InversifyJS best practices and maintains the existing constructor signature.
packages/main/src/plugin/recommendations/recommendations-registry.ts (4)
19-19
: LGTM: InversifyJS imports addedCorrectly imports the dependency injection decorators.
21-30
: LGTM: Import conversions for dependency injectionProperly converts type-only imports to value imports for ExtensionsCatalog, ExtensionLoader, Featured, and IConfigurationRegistry to enable runtime injection.
35-35
: LGTM: Injectable decoratorThe
@injectable()
decorator correctly enables RecommendationsRegistry for Inversify container management.
38-45
: LGTM: Constructor dependency injectionThe
@inject()
decorators are properly applied with correct injection tokens:
IConfigurationRegistry
for configuration managementFeatured
for featured extensionsExtensionLoader
for extension loadingExtensionsCatalog
for extension catalog accessThe implementation maintains the existing constructor interface while enabling dependency injection.
packages/main/src/plugin/tasks/notification-registry.ts (5)
2-2
: LGTM: Copyright year updateStandard maintenance to reflect 2025.
21-21
: LGTM: InversifyJS importsCorrectly adds the necessary dependency injection decorators.
25-27
: LGTM: Import changes for injectionProperly converts type-only imports to value imports for ApiSenderType and TaskManager to support runtime dependency injection.
29-29
: LGTM: Injectable decoratorThe
@injectable()
decorator enables NotificationRegistry to be managed by the Inversify container.
35-38
: LGTM: Constructor parameter injectionThe
@inject()
decorators are correctly applied:
ApiSenderType
for API communicationTaskManager
for task managementThe implementation maintains the existing constructor interface while enabling dependency injection.
packages/main/src/plugin/extension/extension-development-folders.ts (4)
22-22
: LGTM: InversifyJS importsCorrectly adds the necessary dependency injection decorators.
25-30
: LGTM: Import conversions for dependency injectionProperly converts type-only imports to value imports for IConfigurationRegistry, ApiSenderType, and ExtensionAnalyzer to enable runtime injection.
33-33
: LGTM: Injectable decoratorThe
@injectable()
decorator correctly enables ExtensionDevelopmentFolders for Inversify container management.
58-63
: LGTM: Constructor dependency injectionThe
@inject()
decorators are properly applied with correct injection tokens:
IConfigurationRegistry
for configuration managementExtensionAnalyzer
for extension analysisApiSenderType
for API communicationThe implementation maintains the existing constructor interface and complex extension development folder logic while enabling dependency injection.
packages/main/src/plugin/kubernetes/kubernetes-client.ts (1)
69-69
: LGTM! Clean dependency injection implementation.The InversifyJS integration is properly implemented:
- Correct import of DI decorators
- Appropriate conversion from type-only to value imports for runtime injection
- Proper class and constructor parameter decoration
- Injection tokens are consistently used
Also applies to: 77-77, 88-88, 90-91, 170-170, 209-216
packages/main/src/plugin/tasks/task-manager.ts (1)
2-2
: LGTM! Consistent dependency injection implementation.The DI setup follows the same clean pattern as other files in the codebase:
- Proper decorator imports and application
- Correct import conversions for runtime dependency resolution
- Consistent injection token usage
- Copyright year appropriately updated
Also applies to: 20-20, 26-26, 30-32, 34-34, 41-48
packages/main/src/plugin/color-registry.ts (3)
20-20
: LGTM! Import changes correctly support runtime injection.The conversion from type-only imports to value imports for
inject
,injectable
,ApiSenderType
, andConfigurationRegistry
is necessary to support runtime dependency injection.Also applies to: 28-28, 30-30
33-33
: LGTM! Proper InversifyJS class annotation.The
@injectable()
decorator correctly marks this class for dependency injection container management.
35-36
: LGTM! Field declarations correctly updated for guaranteed injection.Removing the optional modifiers (
?
) is appropriate since these dependencies are now always provided via constructor injection.packages/main/src/plugin/provider-registry.ts (3)
56-56
: LGTM! Import changes correctly support runtime injection.The conversion from type-only imports to value imports for the InversifyJS decorators and injection tokens (
ApiSenderType
,ContainerProviderRegistry
,Telemetry
) is necessary for runtime dependency injection.Also applies to: 70-70, 72-72, 76-76
95-95
: LGTM! Proper InversifyJS class annotation.The
@injectable()
decorator correctly marks this critical registry class for dependency injection container management.
160-167
: Verify InversifyJS container bindings for ProviderRegistry dependenciesI didn’t find any
container.bind
calls forApiSenderType
,ContainerProviderRegistry
, orTelemetry
in the repository. Please double-check your main DI setup to ensure these tokens are bound beforeProviderRegistry
is instantiated.• Locate where your Inversify
Container
is created or modules are registered
• Confirm bindings for:
–ApiSenderType
–ContainerProviderRegistry
–Telemetry
packages/main/src/plugin/updater.ts (4)
31-31
: LGTM: Inversify imports added correctly.The import of
inject
andinjectable
decorators from Inversify is correct for enabling dependency injection.
34-46
: LGTM: Import changes support runtime injection.Converting type-only imports to regular imports is necessary to support runtime dependency injection with Inversify. The imports for
CommandRegistry
,ConfigurationRegistry
,MessageBox
,StatusBarRegistry
,TaskManager
, andApiSenderType
are correctly changed.
51-51
: LGTM: Class properly decorated for injection.The
@injectable()
decorator is correctly applied to theUpdater
class to make it available for dependency injection.
63-74
: LGTM: Constructor parameters correctly annotated for injection.All constructor parameters are properly decorated with
@inject()
annotations specifying the correct injection tokens. The dependency injection setup maintains the existing constructor signature and functionality while enabling IoC container management.packages/main/src/plugin/navigation/navigation-manager.ts (4)
20-20
: LGTM: Inversify imports added correctly.The import of
inject
andinjectable
decorators from Inversify is correct for enabling dependency injection.
22-32
: LGTM: Import changes support runtime injection.Converting type-only imports to regular imports is necessary to support runtime dependency injection. The imports for
ApiSenderType
,CommandRegistry
,ContainerProviderRegistry
,ContributionManager
,OnboardingRegistry
,ProviderRegistry
, andWebviewRegistry
are correctly changed.
39-39
: LGTM: Class properly decorated for injection.The
@injectable()
decorator is correctly applied to theNavigationManager
class to make it available for dependency injection.
44-57
: LGTM: Constructor parameters correctly annotated for injection.All constructor parameters are properly decorated with
@inject()
annotations specifying the correct injection tokens. The dependency injection setup maintains the existing constructor signature and functionality while enabling IoC container management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (67)
packages/api/src/configuration/models.ts
(1 hunks)packages/main/src/plugin/api.ts
(1 hunks)packages/main/src/plugin/appearance-init.ts
(1 hunks)packages/main/src/plugin/authentication.ts
(3 hunks)packages/main/src/plugin/autostart-engine.ts
(1 hunks)packages/main/src/plugin/cancellation-token-registry.ts
(2 hunks)packages/main/src/plugin/certificates.ts
(3 hunks)packages/main/src/plugin/cli-tool-registry.ts
(2 hunks)packages/main/src/plugin/close-behavior.ts
(1 hunks)packages/main/src/plugin/color-registry-inject.spec.ts
(1 hunks)packages/main/src/plugin/color-registry-inject.ts
(1 hunks)packages/main/src/plugin/color-registry.ts
(1 hunks)packages/main/src/plugin/command-registry.ts
(3 hunks)packages/main/src/plugin/commands-init.ts
(1 hunks)packages/main/src/plugin/configuration-registry.ts
(3 hunks)packages/main/src/plugin/confirmation-init.ts
(2 hunks)packages/main/src/plugin/container-registry.ts
(5 hunks)packages/main/src/plugin/context/context.ts
(2 hunks)packages/main/src/plugin/contribution-manager.ts
(3 hunks)packages/main/src/plugin/custompick/custompick-registry.ts
(2 hunks)packages/main/src/plugin/dialog-registry.ts
(2 hunks)packages/main/src/plugin/directories.ts
(2 hunks)packages/main/src/plugin/docker/docker-compatibility.ts
(3 hunks)packages/main/src/plugin/editor-init.ts
(1 hunks)packages/main/src/plugin/extension/catalog/extensions-catalog.ts
(2 hunks)packages/main/src/plugin/extension/extension-analyzer.ts
(2 hunks)packages/main/src/plugin/extension/extension-development-folders.ts
(2 hunks)packages/main/src/plugin/extension/extension-loader.ts
(4 hunks)packages/main/src/plugin/extension/extension-watcher.ts
(2 hunks)packages/main/src/plugin/featured/featured.ts
(1 hunks)packages/main/src/plugin/feedback-handler.ts
(2 hunks)packages/main/src/plugin/filesystem-monitoring.ts
(2 hunks)packages/main/src/plugin/icon-registry.ts
(2 hunks)packages/main/src/plugin/image-checker.ts
(2 hunks)packages/main/src/plugin/image-files-registry.ts
(2 hunks)packages/main/src/plugin/image-registry.ts
(4 hunks)packages/main/src/plugin/input-quickpick/input-quickpick-registry.ts
(2 hunks)packages/main/src/plugin/kubernetes/kube-generator-registry.ts
(3 hunks)packages/main/src/plugin/kubernetes/kubernetes-client.ts
(4 hunks)packages/main/src/plugin/learning-center-init.ts
(1 hunks)packages/main/src/plugin/libpod-api-enable/libpod-api-init.ts
(1 hunks)packages/main/src/plugin/menu-registry.ts
(2 hunks)packages/main/src/plugin/message-box.ts
(1 hunks)packages/main/src/plugin/navigation-items-init.ts
(1 hunks)packages/main/src/plugin/navigation/navigation-manager.ts
(1 hunks)packages/main/src/plugin/onboarding-registry.ts
(1 hunks)packages/main/src/plugin/open-devtools-init.ts
(1 hunks)packages/main/src/plugin/provider-registry.ts
(4 hunks)packages/main/src/plugin/proxy.ts
(4 hunks)packages/main/src/plugin/recommendations/recommendations-registry.ts
(1 hunks)packages/main/src/plugin/release-notes-banner-init.ts
(2 hunks)packages/main/src/plugin/safe-storage/safe-storage-registry.ts
(3 hunks)packages/main/src/plugin/statusbar/pin-registry.ts
(2 hunks)packages/main/src/plugin/statusbar/statusbar-providers-init.ts
(1 hunks)packages/main/src/plugin/statusbar/statusbar-registry.ts
(1 hunks)packages/main/src/plugin/tasks/notification-registry.ts
(2 hunks)packages/main/src/plugin/tasks/progress-impl.ts
(3 hunks)packages/main/src/plugin/tasks/task-manager.ts
(2 hunks)packages/main/src/plugin/telemetry/telemetry.ts
(3 hunks)packages/main/src/plugin/terminal-init.ts
(1 hunks)packages/main/src/plugin/tray-icon-color.ts
(1 hunks)packages/main/src/plugin/tray-menu-registry.ts
(2 hunks)packages/main/src/plugin/troubleshooting.ts
(2 hunks)packages/main/src/plugin/updater.ts
(2 hunks)packages/main/src/plugin/view-registry.ts
(2 hunks)packages/main/src/plugin/webview/webview-registry.ts
(3 hunks)packages/main/src/plugin/welcome/welcome-init.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- packages/main/src/plugin/directories.ts
- packages/main/src/plugin/color-registry-inject.ts
- packages/main/src/plugin/configuration-registry.ts
- packages/main/src/plugin/input-quickpick/input-quickpick-registry.ts
- packages/main/src/plugin/color-registry.ts
🚧 Files skipped from review as they are similar to previous changes (61)
- packages/main/src/plugin/cancellation-token-registry.ts
- packages/api/src/configuration/models.ts
- packages/main/src/plugin/troubleshooting.ts
- packages/main/src/plugin/filesystem-monitoring.ts
- packages/main/src/plugin/extension/extension-analyzer.ts
- packages/main/src/plugin/api.ts
- packages/main/src/plugin/view-registry.ts
- packages/main/src/plugin/certificates.ts
- packages/main/src/plugin/kubernetes/kube-generator-registry.ts
- packages/main/src/plugin/close-behavior.ts
- packages/main/src/plugin/statusbar/pin-registry.ts
- packages/main/src/plugin/statusbar/statusbar-registry.ts
- packages/main/src/plugin/statusbar/statusbar-providers-init.ts
- packages/main/src/plugin/confirmation-init.ts
- packages/main/src/plugin/webview/webview-registry.ts
- packages/main/src/plugin/tasks/progress-impl.ts
- packages/main/src/plugin/learning-center-init.ts
- packages/main/src/plugin/icon-registry.ts
- packages/main/src/plugin/terminal-init.ts
- packages/main/src/plugin/message-box.ts
- packages/main/src/plugin/appearance-init.ts
- packages/main/src/plugin/welcome/welcome-init.ts
- packages/main/src/plugin/docker/docker-compatibility.ts
- packages/main/src/plugin/cli-tool-registry.ts
- packages/main/src/plugin/navigation-items-init.ts
- packages/main/src/plugin/context/context.ts
- packages/main/src/plugin/editor-init.ts
- packages/main/src/plugin/extension/extension-watcher.ts
- packages/main/src/plugin/tray-icon-color.ts
- packages/main/src/plugin/image-registry.ts
- packages/main/src/plugin/custompick/custompick-registry.ts
- packages/main/src/plugin/tray-menu-registry.ts
- packages/main/src/plugin/safe-storage/safe-storage-registry.ts
- packages/main/src/plugin/menu-registry.ts
- packages/main/src/plugin/release-notes-banner-init.ts
- packages/main/src/plugin/autostart-engine.ts
- packages/main/src/plugin/extension/extension-development-folders.ts
- packages/main/src/plugin/authentication.ts
- packages/main/src/plugin/image-files-registry.ts
- packages/main/src/plugin/featured/featured.ts
- packages/main/src/plugin/commands-init.ts
- packages/main/src/plugin/open-devtools-init.ts
- packages/main/src/plugin/libpod-api-enable/libpod-api-init.ts
- packages/main/src/plugin/contribution-manager.ts
- packages/main/src/plugin/image-checker.ts
- packages/main/src/plugin/tasks/task-manager.ts
- packages/main/src/plugin/telemetry/telemetry.ts
- packages/main/src/plugin/onboarding-registry.ts
- packages/main/src/plugin/container-registry.ts
- packages/main/src/plugin/recommendations/recommendations-registry.ts
- packages/main/src/plugin/tasks/notification-registry.ts
- packages/main/src/plugin/extension/catalog/extensions-catalog.ts
- packages/main/src/plugin/proxy.ts
- packages/main/src/plugin/kubernetes/kubernetes-client.ts
- packages/main/src/plugin/dialog-registry.ts
- packages/main/src/plugin/feedback-handler.ts
- packages/main/src/plugin/command-registry.ts
- packages/main/src/plugin/navigation/navigation-manager.ts
- packages/main/src/plugin/updater.ts
- packages/main/src/plugin/provider-registry.ts
- packages/main/src/plugin/extension/extension-loader.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/main/src/plugin/color-registry-inject.spec.ts (1)
packages/main/src/plugin/api.ts (2)
ApiSenderType
(21-21)ApiSenderType
(22-25)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: typecheck
- GitHub Check: smoke e2e tests (production)
- GitHub Check: smoke e2e tests (development)
- GitHub Check: unit tests / macos-15
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: unit tests / windows-2022
- GitHub Check: macOS
- GitHub Check: build website
- GitHub Check: Linux
- GitHub Check: linter, formatters
- GitHub Check: Windows
🔇 Additional comments (2)
packages/main/src/plugin/color-registry-inject.spec.ts (2)
41-43
: LGTM: Proper Inversify container configuration.The container bindings correctly map the injection tokens to their implementations, with appropriate scoping for the InjectableColorRegistry.
28-31
: LGTM: ApiSenderType mock matches interface.The mock correctly implements the
ApiSenderType
interface with propersend
andreceive
method signatures matching the definition frompackages/main/src/plugin/api.ts
.
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
it will allow to delegate to inversify the creation of objects related to #12640 Signed-off-by: Florent Benoit <fbenoit@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM codewise
const container = new Container(); | ||
container.bind(ApiSenderType).toConstantValue(apiSender); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this bind
part of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the issue is that without importing Container, it's not importing 'reflect-metadata' which is a required package for decorators
if I want to just add the direct import, I need to add a dependency to reflect-metadata package
so I've added the minimal import of container (and if I only use new Container()
then I have a linter failure because object is not used)
so this is the only way I found to make linter happier and then preparing all the binding that will come in another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What does this PR do?
it will allow to delegate to inversify the creation of objects
Screenshot / video of UI
What issues does this PR fix or reference?
related to #12640
How to test this PR?
should be a no-op