Aman-Mittal opened a new issue, #442:
URL: https://github.com/apache/fineract-backoffice-ui/issues/442

   Dependabot has opened one PR per package. Angular does not ship one package 
at a time, so these
   have to land together or not at all. This is that PR.
   
   **This is a good first issue.** It is a dependency bump with a well-defined 
done condition, and CI
   tells you whether you got it right. What it is *not* is "click merge on four 
PRs" — the whole point
   of the task is deciding which of them belong together.
   
   ## The four open PRs
   
   | PR | Bump | Verdict |
   | --- | --- | --- |
   | #313 | `@angular/router` 22.0.7 → 22.1.0 | include |
   | #316 | `@angular/platform-browser` 22.0.7 → 22.1.0 | include |
   | #207 | `@ngx-translate/core` 17.0.0 → 18.0.0 | **separate issue** — a 
major, see below |
   | #312 | `typescript` 6.0.3 → 7.0.2 | **close it** — see below |
   
   ## What to do
   
   Bump **every** `@angular/*` package to 22.1.0 in one commit, not just the 
two Dependabot happened
   to open PRs for. Today all nine sit at 22.0.7:
   
   ```
   @angular/animations   @angular/common   @angular/compiler   
@angular/compiler-cli
   @angular/core         @angular/forms    @angular/platform-browser
   @angular/router       @angular/build
   ```
   
   `@angular/cdk` (22.0.6) tracks its own line and does not have to match 
exactly — check what
   22.1-compatible version it wants rather than assuming.
   
   The reason for lockstep is not superstition: `@angular/[email protected]` 
declares
   `"@angular/compiler": "22.1.0"` as an **exact** peer. Move the compiler 
without the compiler-cli
   and npm has no valid tree.
   
   A subtlety worth understanding, because it explains why CI is green today: 
the declared ranges are
   already `^22.0.7`, which *permits* 22.1.0. The lockfile is what pins 22.0.7. 
So this PR is mostly a
   lockfile change, and `npm ci` is what makes it real — which is also why `npm 
install` alone is not
   enough to prove anything.
   
   ### Steps
   
   ```bash
   git checkout -b chore/angular-22.1.0
   npm install --save-exact=false \
     @angular/[email protected] @angular/[email protected] @angular/[email protected] 
\
     @angular/[email protected] @angular/[email protected] 
@angular/[email protected] \
     @angular/[email protected]
   npm install --save-dev @angular/[email protected] @angular/[email protected]
   rm -rf node_modules && npm ci        # prove the lockfile resolves from 
scratch
   ```
   
   Then the full gate — the e2e suites matter here more than usual, because a 
framework minor moves
   change detection and routing:
   
   ```bash
   npm run lint && npm run format:check
   npm run test -- --watch=false        # Karma
   npm run test:unit                    # Vitest
   npm run typecheck:e2e
   npx playwright test --project=mocked
   npx playwright test --project=mobile
   ```
   
   Close #313 and #316 with a note pointing at your PR.
   
   ## Why TypeScript 7 is excluded
   
   `@angular/[email protected]` declares:
   
   ```json
   "peerDependencies": { "typescript": ">=6.0 <6.1", "@angular/compiler": 
"22.1.0" }
   ```
   
   TypeScript 7.0.2 is outside that range. Angular 22 does not support it, so 
#312 cannot land with
   this work or after it — it needs an Angular release that widens the peer. 
Please close #312 with
   that reason rather than leaving it to rot; Dependabot will reopen when there 
is a version that
   fits.
   
   Do not be tempted by `--legacy-peer-deps`. The peer range is the compiler 
telling you which
   TypeScript AST it knows how to read.
   
   ## Why ngx-translate is a separate issue, and what the adapter has to do 
with it
   
   #207 is a **major**, and it is not a beginner task. Two reasons.
   
   **It does not travel alone.** `@ngx-translate/http-loader` is also at 17.0.0 
and would have to move
   to 18.0.0 with it. Worth checking first whether it needs to move at all — 
#437 replaced
   `provideTranslateHttpLoader` with `DeploymentTranslateLoader`, so 
`http-loader` may now be an
   unused dependency that should simply be dropped. Confirm with a grep before 
either.
   
   **The adapter boundary that should contain it does not, yet.** ADR-0003 puts 
ngx-translate behind
   the `I18N` token in `src/app/core/adapters/i18n/` precisely so a major like 
this is a four-file
   change. In practice:
   
   ```
   381 files import '@ngx-translate/core' directly
     2 of them are inside core/adapters/
   ```
   
   `npm run lint` already forbids that import outside the adapter — the other 
379 are recorded in
   `eslint-suppressions.json`, which is why the build is green. So the boundary 
exists on paper and
   the blast radius is still the whole codebase.
   
   Most of those imports are `TranslateModule` pulled in for the `| translate` 
pipe (765 uses), which
   the adapter already offers as `| appTranslate`. That is the shape of the 
work, and it is worth
   doing *before* v18 rather than during it:
   
   1. Migrate templates from `| translate` to `| appTranslate`, dropping 
`TranslateModule` from each
      component's `imports`. Mechanical, reviewable, shrinks the suppression 
list — and
      `npm run lint:prune` keeps that list honest as it goes.
   2. Move the 85 `TranslateService` injections onto the `I18N` token. Anything 
`I18nAdapter` cannot
      express is a finding: either the contract is missing something, or that 
call site is doing
      something it should not.
   3. *Then* bump to v18, and the change is confined to
      `ngx-translate-i18n.adapter.ts`, `deployment-translate.loader.ts` and 
`app.config.ts`.
   
   Done in that order, v18 is a small PR. Done the other way round it is a 
381-file PR that nobody can
   review, which is how a library upgrade turns into a six-month stall.
   
   ## Acceptance criteria
   
   - [ ] All nine `@angular/*` packages at 22.1.0 in `package.json` **and** 
`package-lock.json`
   - [ ] `npm ci` succeeds from a clean `node_modules` with no peer warnings
   - [ ] Lint, format, both unit suites, `typecheck:e2e`, and the `mocked` and 
`mobile` Playwright
         projects all pass
   - [ ] No `--legacy-peer-deps`, no `overrides` block, no `resolutions`
   - [ ] #313 and #316 closed with a pointer to the new PR
   - [ ] #312 closed, citing the compiler-cli peer range
   - [ ] A follow-up issue opened for the ngx-translate v18 migration, 
referencing the adapter work
         above
   
   ## Background
   
   - `DOCS/adr/0003-adapter-boundary.md` — why the `I18N` token exists
   - `DOCS/ADAPTERS.md` — the tokens and what they replace
   - `DOCS/LINT_POLICY.md` — the suppression backlog, and the rule that it may 
only shrink
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to