Skip to content

[#74950] Angular: Migration to the inject function#23206

Open
myabc wants to merge 1 commit into
devfrom
code-maintenance/74950-angular-inject-function
Open

[#74950] Angular: Migration to the inject function#23206
myabc wants to merge 1 commit into
devfrom
code-maintenance/74950-angular-inject-function

Conversation

@myabc
Copy link
Copy Markdown
Contributor

@myabc myabc commented May 13, 2026

Ticket

https://community.openproject.org/wp/74950

What are you trying to accomplish?

Migrate Angular constructor-based dependency injection to the inject() function across the frontend codebase. This is a mechanical refactoring that removes ~2 200 lines of constructor boilerplate while aligning with the Angular team's recommended DI pattern.

What approach did you choose and why?

Used the official Angular schematic as the initial pass:

cd frontend && npx ng generate @angular/core:inject-migration \
  --path ./ \
  --migrate-abstract-classes \
  --backwards-compatible-constructors=false \
  --non-nullable-optional=false

Manual follow-up was needed for:

  • Missed child classes whose parents lost their constructor (e.g. GlobalSearchTabsComponent, ViewSettingsModalComponent)
  • Factory-instantiated services (ProjectCache, viewerBridgeServiceFactory) that run outside an injection context — kept constructor params or wrapped in runInInjectionContext()
  • OpAutocompleterService moved to providedIn: 'root' so subclasses can inherit it without a component-level provider
  • Eslint fixes on changed lines (type-annotation spacing, unused imports, anyunknown, ||??)

Merge checklist

  • Added/updated tests
  • Added/updated documentation in Lookbook (patterns, previews, etc)
  • Tested major browsers (Chrome, Firefox, Edge, ...)

@myabc myabc requested a review from Copilot May 13, 2026 18:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@myabc myabc force-pushed the code-maintenance/74950-angular-inject-function branch from 0470cd5 to 8ac7844 Compare May 13, 2026 19:07
Migrates Angular constructor-based dependency injection to the
`inject()` function. The initial pass used the Angular schematic;
manual follow-up handled abstract classes, inheritance-sensitive
constructors, and call sites that still instantiate services
directly.

Schematic command:

  cd frontend && npx ng generate @angular/core:inject-migration \
    --path ./ \
    --migrate-abstract-classes \
    --backwards-compatible-constructors=false \
    --non-nullable-optional=false

https://community.openproject.org/wp/74950
@myabc myabc force-pushed the code-maintenance/74950-angular-inject-function branch from 8ac7844 to e8825ba Compare May 13, 2026 20:32
@myabc myabc requested a review from Copilot May 13, 2026 20:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@myabc myabc added maintenance javascript Pull requests that update Javascript code labels May 13, 2026
@myabc myabc marked this pull request as ready for review May 13, 2026 20:38
@github-actions
Copy link
Copy Markdown

Caution

The provided work package version does not match the core version

Details:

Please make sure that:

  • The work package version OR your pull request target branch is correct

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

Labels

javascript Pull requests that update Javascript code maintenance needs review

Development

Successfully merging this pull request may close these issues.

3 participants