Stop establishing unnecessary reactive dependencies

[imajus]

The problem I have faced in my current Meteor project is clearly demonstrated in this simple example project: https://github.com/imajus/blaze-reactive-loop-example

While this doesn't cause visible issues in most cases, in some cases, like the one presented in this repository, it can lead to unexpected and hard-to-track issues. Additionally, it adds unnecessary overhead to the reactive computation system, which may result in unnecessary re-rendering.

Proposed solution

Since the behavior in question is definitely not intended and potentially harmful, I believe the best approach would be to prevent event handlers from establishing new reactive dependencies if they are called from any reactive computation.

[CLAassistant]

All committers have signed the CLA.

[jankapunkt]

@imajus thank you for this PR! Would you mind checking the tests? There seem to be at least one failed test, indicating that this may be a breaking change.

[imajus]

@jankapunkt thanks for a guidance. There was, in fact, an error in my initial implementation, which is now fixed. Hail to the automated testing!

[jankapunkt]

Generally approved from my end. @StorytellerCZ do you have something to add? If not we can merge this. @imajus is there a way to test this from a UnitTest perspective? I've looked into the the example and I would like to avoid detecting, whether we are in a loop (if it fails it would consume all our free CI time).

[StorytellerCZ]

LGTM

[imajus]

@jankapunkt Thanks for approving the PR.
For the automated test you may choose to merge the following PR to this branch first, which will make currently failing test there to pass, and only then merge this PR.

What I have introduced there is more like an integration test, but it is designed the way that it won't cause an infinite loop in case of a failure. I think that it is the most practical way of testing this issue.

If you'd like I could also cover Template.events with unit tests, ensuring that no reactive dependencies are established in the event handler wrappers there. I just need a confirmation that such test would make sense in the system.

[jankapunkt]

@StorytellerCZ should we target 2.6.2 or 3.0.0 with this?

[StorytellerCZ]

Let's do 2.6.2 and release it with the changes earlier.