Skip to content
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

I18nPipe does not update in response to AlainI18NService.change events #2529

Open
goldsam opened this issue Nov 7, 2024 · 4 comments
Open

Comments

@goldsam
Copy link

goldsam commented Nov 7, 2024

Reproduction link

The reference project on stackblitz would not finish installing npm dependencies, so I was not able to create a minimal reproduction. codesandbox was consistently hanging and crashing my browser (chrome and Edge on two different machines).

Steps to reproduce

When using ChangeDetectionStrategy.OnPush, components which use the I18nPipe are not updated when the AlainI18NService.change observable emits a new value.

What is expected?

Components should update when AlainI18NService.change emits.

What is actually happening?

Since the pipe is declared as "pure", AlainI18NService change events are ignored.

Environment Info
ng-alain 18.3.0
Browser n/a

The implementation from ngx-translate can be used as a reference.

A proper implementation would do the following:

  • Set the pure attribute to false in the decorator
  • Cache the translated value returned by AlainI18NService.fanyi
  • Subscribe to the AlainI18NService.change observable and recompute the cache translation appropriately.
  • Unsubscribe to AlainI18NService.change in an ngOnDestroy callback
@cipchk
Copy link
Member

cipchk commented Nov 8, 2024

In fact, NG-ALAIN did initially rely on ngx-translate as part of its internationalization.

However, over time, it was discovered that if internationalization data can be confirmed at the Angular startup, the implementation method of ngx-translate is not as developer-friendly.

Therefore, NG-ALAIN provides a simplified approach, which assumes that internationalization data is always ready to be used. Of course, aside from relying on APP_INITIALIZER, current route guards can also address the issue of establishing internationalization data with delayed routes.

For refreshing internationalization, I believe using location.reload() might slightly affect the experience, but from a development perspective, it is more worthwhile.

NG-ALAIN does not strictly enforce using the default implementation and can fully utilize DI rules to override this default behavior.

@goldsam
Copy link
Author

goldsam commented Nov 8, 2024

Do you mean override the Pipe itself?

@goldsam
Copy link
Author

goldsam commented Nov 14, 2024

The contract seems to be that AlainI18NService.change is an event fired when the AlainI18NService state changes (selected language or current set of translations), yet this is not being honored by the current I18nPipe. The simple fact is that I18nPipe is not a pure since it depends on AlainI18NService state. This is a bug that should be fixed. The UX of reloading is ugly and unnecessary.

A very simple caching strategy (like the one linked to above in ngx-translate) is cheap to evaluate and would have negligible impact on runtime performance. The ngx-translate license is very permissive. You could event borrow their implementation as long as you attribute it properly.

@cipchk
Copy link
Member

cipchk commented Dec 1, 2024

Angular's dependency injection can help you override NG-ALAIN's default behavior, including I18nPipe. In fact, you only need to ensure the implementation of ALAIN_I18N_TOKEN (which internally provides a default behavior via AlainI18nBaseService). Since the entire @delon/* library involves a lot of internationalization, everything depends on ALAIN_I18N_TOKEN. How you implement internationalization is entirely up to you. Otherwise, NG-ALAIN’s default implementation will be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants