Run backtracking assertion only in debug mode#14856
Run backtracking assertion only in debug mode#14856elwayman02 wants to merge 2 commits intoemberjs:masterfrom elwayman02:backtracking-debug-only
Conversation
This wraps the `assertNotRendered` check in `runInDebug` so that it won't execute in production, even when either of the flags are enabled.
| isEnabled('ember-glimmer-allow-backtracking-rerender')) { | ||
| assertNotRendered(obj, keyName, meta); | ||
| isEnabled('ember-glimmer-allow-backtracking-rerender')) { | ||
| runInDebug(() => { assertNotRendered(obj, keyName, meta) }); |
There was a problem hiding this comment.
perhaps move the runInDebug to wrap the if statement too?
There was a problem hiding this comment.
Seems reasonable
| isEnabled('ember-glimmer-allow-backtracking-rerender')) { | ||
| assertNotRendered(obj, keyName, meta); | ||
| } | ||
| assertNotRendered(obj, keyName, meta) |
There was a problem hiding this comment.
So I dug into this a bit, and it turns out that this isn't the right fix. Specifically, in the case where ember-glimmer-allow-backtracking-rerender is enabled we have to actually call assertNotRendered or we will not trigger a rerender when we hit this backtracking. In other words, wrapping this in runInDebug does avoid an error (due to a bug in assertNotRendered) but it also breaks the ability for rerendering to happen.
I submitted #14876 to address.
|
I'm going to close this in favor of the refactoring in #14876. Thank you for pointing out the issue and taking a stab at fixing! |
|
@rwjblue we're trying to fix the issue in this ticket for 2.10.2. I see that your PR is against master which has some more changes beyond this, so some methods don't exist or have been changed. It's hard to figure out if there's a simpler fix to get this working in 2.10.2. |
|
Its likely that my commit in #14876 applies cleanly to release-2-10 branch. |
|
There were conflicts but they aren't that bad. I'll try it once more and run through tests. |
6 participants
This wraps the
assertNotRenderedcheck inrunInDebugso that it won't execute in production, even when either of the flags are enabled.