Skip to content
This repository was archived by the owner on May 25, 2021. It is now read-only.

Sanity for AppRef.isStable observable #1417

Merged
merged 3 commits into from
Nov 19, 2019
Merged

Sanity for AppRef.isStable observable #1417

merged 3 commits into from
Nov 19, 2019

Conversation

feeloor
Copy link
Contributor

@feeloor feeloor commented Nov 15, 2019

In larger applications, we sometimes encounter the appRef.isStable observable to fire constantly which is causing us to constantly send pings and updates, this in turn is making larger applications almost unusable.
To prevent this I've added a sanity check to make sure that we only emit at the most twice per second.

Related issues: #1320 possibly #1402

@feeloor feeloor self-assigned this Nov 15, 2019
@feeloor feeloor force-pushed the sanity-for-observable branch from 3667d8c to 3ff7f30 Compare November 18, 2019 10:47
const roots = collectRoots();
if (roots.length) {
let sanity;
const sanityThreshold = 0.5 * 1000; // 0.5 seconds
Copy link
Member

Choose a reason for hiding this comment

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

@feeloor Please add a comment explaining what is going on. Rest looks good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sumitarora I've now added a few comments. 👍

@feeloor feeloor force-pushed the sanity-for-observable branch from 31989de to 462ccb4 Compare November 19, 2019 08:50
@feeloor feeloor merged commit 010376b into master Nov 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants