-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Refactor: Use Angular inject() for Dependency Injection Across All Modules - Issue 23231 #23262
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
Eliminated unused injections of RestService, Router, and ViewContainerRef from MultiTenancyService, LoaderBarComponent, and LoadingDirective respectively to clean up the code and improve maintainability.
Updated the LocaleId class to use Angular's inject() function for obtaining the LocalizationService instead of constructor injection. Removed the deps array from the LocaleProvider as it is no longer needed.
Updated PageToolbarComponent and LazyStyleHandler initialization to remove the unused Injector parameter from their constructors and factory functions. This simplifies instantiation and aligns with recent refactoring.
...cks/packages/components/extensible/src/lib/components/grid-actions/grid-actions.component.ts
Outdated
Show resolved
Hide resolved
...cs/src/commands/api/files-service/proxy/__namespace@dir__/__name@kebab__.service.ts.template
Show resolved
Hide resolved
npm/ng-packs/packages/theme-shared/src/lib/directives/ellipsis.directive.ts
Outdated
Show resolved
Hide resolved
npm/ng-packs/packages/theme-shared/src/lib/components/loader-bar/loader-bar.component.ts
Outdated
Show resolved
Hide resolved
npm/ng-packs/packages/theme-shared/src/lib/directives/loading.directive.ts
Outdated
Show resolved
Hide resolved
npm/ng-packs/packages/theme-shared/src/lib/utils/date-parser-formatter.ts
Show resolved
Hide resolved
sumeyyeKurtulus
left a comment
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.
Hello @fahrigedik , it looks nice and neat other than the points I mentioned above.
Eliminated unused imports from several components and directives to improve code clarity and reduce bundle size. Added an explicit constructor to DateParserFormatter for consistency. No functional changes were made.
yagmurcelk
left a comment
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.
Updated MockPermissionService and MockRestService to call their super constructors without arguments, aligning with possible changes in the base class constructors. This simplifies the instantiation logic and removes unnecessary dependencies from the mock implementations.
yagmurcelk
left a comment
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.
yagmurcelk
left a comment
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.
yagmurcelk
left a comment
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.
|
Hi @yagmurcelk These errors occur because the scripts are not loaded and I don't get them. would you like someone else to test it? |







Description
Resolves #23231
This PR refactors all Angular services, components, and directives in the repository to use the Angular v14+ inject() function for dependency injection instead of constructor-based DI. All DI dependencies are now initialized as class properties using inject(), and constructors have been removed or simplified where appropriate. This change aligns the codebase with Angular's latest best practices for dependency injection, improves code readability, and prepares the project for future Angular versions.
Highlights:
All services, components, and directives now use inject() for DI.
Constructors that only existed for DI have been removed.
No functional or API changes were made; only the DI pattern was updated.
No breaking changes for consumers unless they relied on constructor injection in extended classes.
No screenshots or GIFs are required as this is a code refactor.
Checklist
How to test it?
Run the application and ensure all features work as before.
Pay special attention to areas where services, components, or directives are injected.
Run all existing unit and integration tests to confirm there are no regressions.
for @sumeyyeKurtulus
Migration Options & Rationale
Migration Path: abp
Choice: Migrate entire project
Rationale: Ensure consistency across all code and avoid mixed DI patterns.
Abstract Classes Migration: Yes
Backwards Compatible Constructors: No
Optional Inject Non-nullable: Yes