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

Updated change detection strategy to onPush for greater control #40

Merged
merged 1 commit into from
Jun 9, 2016
Merged

Updated change detection strategy to onPush for greater control #40

merged 1 commit into from
Jun 9, 2016

Conversation

elliott-davis
Copy link
Contributor

This allows the component to work if it is in an onPush environment or a default environemnt

@michaelbromley
Copy link
Owner

Thanks for the PR. I'll be able to properly evaluate it later in the week. By the way, did you intentionally compile the TS without comments?

@elliott-davis
Copy link
Contributor Author

Thanks! I totally did not - it's probably a byproduct of my global tsconfig. Do you prefer just a source commit without the compiled js?

@michaelbromley
Copy link
Owner

Yeah just the source commit is sufficient, since I always need to build everything (docs and lib) anyway after any changes are made. Did you manage to get the tests working with these changes?

@elliott-davis
Copy link
Contributor Author

Cool, I'll get the commit amended. I ran the tests and they all passed but 2 were skipped. Is that normal for what you usually see?

@michaelbromley
Copy link
Owner

Yes sounds right. Those two tests are currently skipped until a particular bug in Angular gets fixed.

@michaelbromley
Copy link
Owner

Just done some testing with this change - absolutely incredible improvement in efficiency: I tested the demo app and performed a series of interactions with the component. In the old version, the change detector gets triggered 535 times. With your changes, only 18!

Thanks!

@michaelbromley michaelbromley merged commit 9a8ce6f into michaelbromley:master Jun 9, 2016
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