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.

Sign up for GitHub

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

Jump to bottom

Deprecate EventEmitter #1513

Closed
JsantosDK opened this issue Dec 27, 2024 · 3 comments
Closed

Deprecate EventEmitter #1513

JsantosDK opened this issue Dec 27, 2024 · 3 comments

Comments

@JsantosDK
Copy link

JsantosDK commented Dec 27, 2024 •
edited
Loading

Current behavior

The library utilizes EventEmitter to emit changes to the selected language.

Expected behavior

The library should rely on a Subject instead, given that these are the most adequate to the use case.

What is the motivation / use case for changing the behavior?

The EventEmitter is made to be utilized with components and directives, as per original angular team instruction.
Furthermore, with the start of moving from @Output/ @Input decorators to signals, output/ input respectively, the usage of the EventEmitter seems inadequate and possible to causes issues due to lack of support in the future.

How do you think we should implement this?

Migrate the onLangChange, onDefaultLangChange and the onTranslationChange from an EventEmitterto a Subject.

I'm ok with forking the repo and doing the changes myself, however, I would like confirmation that is ok before putting work into it.
@CodeAndWeb
Copy link
Member

I am fine with changing this - however have not the time to work in it myself right now.
If you can invest the time, it would be great!

@JsantosDK JsantosDK mentioned this issue Jan 14, 2025
Closed
@JsantosDK
Copy link
Author

Sorry for the delay, I opened #1517 with the changes needed.

CodeAndWeb added a commit that referenced this issue Feb 21, 2025
@CodeAndWeb
Refactoring onTranslationChange, onDefaultLangChange, onLangChange in…
5ebd7a5 
… TranslateStore to use better encapsulation. From outside, these event handlers are now visible as Observables and not EventEmitters to prevent users from outside calling emit().

Internally now using Subjects instead of EventEmitters.

#1513
@CodeAndWeb
Copy link
Member

5ebd7a5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

task: deprecates EventEmitter and replaces it with Subject JsantosDK/core
2 participants
@CodeAndWeb @JsantosDK