[ACS-11213] Reverted ngx translate upgrade#11702
Conversation
AleksanderSklorz
requested review from
DenysVuika and
popovicsandras
as code owners
There was a problem hiding this comment.
Pull request overview
This PR reverts a previous upgrade of @ngx-translate/core from v17 back to v16, addressing issues introduced by that upgrade. The revert includes restoring v16-compatible API usage throughout the codebase.
Changes:
- Downgrade
@ngx-translate/coredependency from^17.0.0to^16.0.4in root and librarypackage.jsonfiles. - Replace v17-specific APIs (
provideTranslateLoader,setFallbackLang,getFallbackLang) with v16-compatible equivalents (TranslateLoaderprovider objects,setDefaultLang,defaultLang). - Update test files to use v16-compatible module setup patterns.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
package.json |
Reverts root @ngx-translate/core to ^16.0.4 |
lib/core/package.json |
Reverts core lib peer dependency to >=16.0.0 |
lib/content-services/package.json |
Reverts content-services peer dependency to >=16.0.0 |
lib/insights/package.json |
Reverts insights peer dependency to >=14.0.0 |
lib/process-services/package.json |
Reverts process-services peer dependency to >=14.0.0 |
lib/process-services-cloud/package.json |
Reverts process-services-cloud peer dependency to >=14.0.0 |
lib/core/src/lib/core.module.ts |
Replaces provideTranslateLoader with inline TranslateLoader provider object |
lib/core/src/lib/translation/provide-i18n.ts |
Replaces provideTranslateLoader/fallbackLang with inline provider/defaultLanguage |
lib/core/src/lib/translation/translation.service.ts |
Reverts to setDefaultLang, getTranslation, and manual onTranslationChange emit |
lib/core/src/lib/translation/translate-loader.spec.ts |
Updates test setup to v16-compatible API |
lib/core/src/lib/translation/translation.service.spec.ts |
Reverts to TranslateModule.forRoot setup |
lib/core/src/lib/translation/provide-i18n.spec.ts |
Updates assertions from getFallbackLang() to defaultLang property |
lib/core/src/lib/testing/noop-translate.module.ts |
Removes TranslateLoader implementation from NoopTranslationService |
| this.translate.onTranslationChange.next({ | ||
| lang, | ||
| translations: this.customLoader.getFullTranslationJSON?.(lang) ?? {} | ||
| }); |
There was a problem hiding this comment.
Directly calling .next() on onTranslationChange (which is a public EventEmitter/Subject from TranslateService) bypasses the internal state management of TranslateService. The previous implementation used this.translate.setTranslation(lang, translations, true), which properly merges translations into the internal store and then fires the event. With this change, the event fires but the translations are not actually stored in TranslateService's internal map, so calls like translateService.instant(key) will not reflect the merged translations from the custom loader.
The v16-compatible approach should call this.translate.setTranslation(lang, translations, true) as before, which both stores the translations and emits the change event internally.
| this.translate.onTranslationChange.next({ | |
| lang, | |
| translations: this.customLoader.getFullTranslationJSON?.(lang) ?? {} | |
| }); | |
| const translations = this.customLoader.getFullTranslationJSON?.(lang) ?? {}; | |
| this.translate.setTranslation(lang, translations, true); |
| useExisting: TranslateLoaderService, | ||
| deps: [HttpClient] |
There was a problem hiding this comment.
The deps field is not applicable when using useExisting, since useExisting creates an alias to an already-existing provider rather than instantiating a new one. The deps array is only meaningful for useClass or useFactory. Including it here is misleading and may cause unexpected behavior depending on the DI framework version. The deps entry should be removed when using useExisting.
| useExisting: TranslateLoaderService, | |
| deps: [HttpClient] | |
| useExisting: TranslateLoaderService |
| provideTranslateService({ | ||
| loader: provideTranslateLoader(NoopTranslationService) | ||
| loader: { | ||
| provide: TranslateLoader, | ||
| useClass: NoopTranslationService | ||
| } |
There was a problem hiding this comment.
NoopTranslationService no longer implements TranslateLoader (the getTranslation method was removed) but is still registered as the TranslateLoader provider. TranslateService will call currentLoader.getTranslation() internally and will fail at runtime because the method no longer exists on NoopTranslationService. Either keep the getTranslation(): Observable<TranslationObject> method on NoopTranslationService, or use a separate minimal class that implements TranslateLoader for this provider.
|
AleksanderSklorz
deleted the
ACS-11213-Upgrade-ngx-translate-to-v17-revert
branch
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x")
What is the current behaviour? (You can also link to an open issue here)
https://hyland.atlassian.net/browse/ACS-11213
What is the new behaviour?
Reverted https://hyland.atlassian.net/browse/ACS-11213
Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: