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

Fix multiple subscriptions to same observables on form activation #299

Merged
merged 1 commit into from
Dec 16, 2021

Conversation

alekart
Copy link
Contributor

@alekart alekart commented Dec 16, 2021

Fixed an issue where some observables were subscribed multiple times each time the activateForm method is called.

The activateForm method is called in setValues method which is called when the form config is updated causing emitters (onChanges, changeDetector, isValid, validationErrors) to emit at least twice after first form init and the number of emits increases each time the form config is updated.
When config is changed all emitters emit for each input present in the layout, as it was not unsubscribed a new subscription
is created on each iteration...

Capture d’écran 2021-12-16 à 13 35 39

@netlify
Copy link

netlify bot commented Dec 16, 2021

✔️ Deploy Preview for ajsf ready!

🔨 Explore the source changes: 043e4f6

🔍 Inspect the deploy log: https://app.netlify.com/sites/ajsf/deploys/61bb33f9ec7039000a99704c

😎 Browse the preview: https://deploy-preview-299--ajsf.netlify.app/

@codecov
Copy link

codecov bot commented Dec 16, 2021

Codecov Report

Merging #299 (043e4f6) into main (880d9c2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #299   +/-   ##
=======================================
  Coverage   20.97%   20.97%           
=======================================
  Files          10       10           
  Lines         906      906           
  Branches      363      363           
=======================================
  Hits          190      190           
  Misses        657      657           
  Partials       59       59           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 880d9c2...043e4f6. Read the comment docs.

Copy link
Owner

@hamzahamidi hamzahamidi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for help @alekart . I will publish this under 0.6.0-beta.1 if that's ok with you

@alekart
Copy link
Contributor Author

alekart commented Dec 16, 2021

Thanks for help @alekart . I will publish this under 0.6.0-beta.1 if that's ok with you

Thats OK for me if it's ok for you. I use a personal fork because the project I use for is still in Angular 9, but this fix seems to be a nice performances enhancement.

@hamzahamidi hamzahamidi merged commit 7abd20b into hamzahamidi:main Dec 16, 2021
@alekart alekart deleted the upstream/fix-multi-sub branch December 16, 2021 21:34
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

Successfully merging this pull request may close these issues.

2 participants