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

abstract-control (angular) state subscription is not cleared #2354

Closed
Ketec opened this issue Jul 8, 2024 · 2 comments · Fixed by #2377
Closed

abstract-control (angular) state subscription is not cleared #2354

Ketec opened this issue Jul 8, 2024 · 2 comments · Fixed by #2377
Assignees
@LukasBoll
Labels
angular bug
Milestone
3.4.1

Comments

@Ketec
Copy link

Ketec commented Jul 8, 2024

Describe the bug

https://github.com/eclipsesource/jsonforms/blob/master/packages/angular/src/library/abstract-control.ts
ngOnDestroy does unsubscribe, but this.jsonFormsService.$state.subscribe is never actually assigned to the variable.

So every schema change adds new subscriptions and old ones still remain active and trigger.

Expected behavior

removed controls are unsubscribed

Steps to reproduce the issue

Screenshots

No response

Which Version of JSON Forms are you using?

3.3.0

Framework

Angular

RendererSet

Other (please specify in the Additional context field)

Additional context

No response
@lucas-koehler
Copy link
Contributor

Hi @Ketec , thanks for the report. That indeed seems like a bug. Would you like to contribute a fix?

@lucas-koehler lucas-koehler added this to the 4.x milestone Jul 8, 2024
@Ketec
Copy link
Author

Ketec commented Jul 8, 2024

I'm not set up to run it locally for tests ( or even how it is set up here).
An open question also would be about the potential use cases with custom renderers that may extend this and use this.subscription for other observables.

It likely needs a separate property to ensure it's backwards compatible.

@sdirix sdirix modified the milestones: 4.x, 3.4 Aug 20, 2024
@lucas-koehler lucas-koehler modified the milestones: 3.4, 4.0 Aug 28, 2024
LukasBoll added a commit to LukasBoll/jsonforms that referenced this issue Sep 15, 2024
@LukasBoll
fix(angular): unsubscribe from jsonForm state subscription
1ca3aa7 
Previously, the subscription to the jsonForm state in Angular was never unsubscribed.
With this commit, all subscriptions will be unsubscribed when the component is destroyed.

Closes eclipsesource#2354
@LukasBoll LukasBoll mentioned this issue Sep 15, 2024
Merged
@sdirix sdirix modified the milestones: 4.0, 3.4.1 Oct 9, 2024
lucas-koehler added a commit that referenced this issue Oct 10, 2024
@LukasBoll @lucas-koehler
angular: Unsubscribe from state subscriptions on renderer destroy (#2377
1feaf0b 
)

Previously, subscriptions to the JSON Forms state in Angular were never removed when renderer components are destroyed.
With this commit, all subscriptions will be unsubscribed when the component is destroyed.
The base renderer offers an addSubscription method to register one or multiple subscriptions
to automatically be unsubscribed on destroy. With this, extending renderers do not need to
implement the unsubscribe again.

Closes #2354

Co-authored-by: Lucas Koehler <lkoehler@eclipsesource.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@LukasBoll LukasBoll

Labels
angular bug
Projects
None yet
Milestone
3.4.1
Development

Successfully merging a pull request may close this issue.

fix(angular): unsubscribe from jsonForm state subscription LukasBoll/jsonforms
4 participants
@lucas-koehler @sdirix @LukasBoll @Ketec