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

Improve "Backtracking re-render" assertion #14709

Closed
GavinJoyce opened this issue Dec 11, 2016 · 6 comments · Fixed by #14723
Closed

Improve "Backtracking re-render" assertion #14709

GavinJoyce opened this issue Dec 11, 2016 · 6 comments · Fixed by #14723

Comments

@GavinJoyce
Copy link
Member

GavinJoyce commented Dec 11, 2016

I've a few backtracking re-render issues to solve in my app and I'm interested in improving the assertions to make this easier for me and others. This issue will serve as a place to capture any discussion or progress.

The current assertion message for this example looks like:

Assertion Failed: You modified model.name twice on [object Object] in a single render. This was unreliable and slow in Ember 1.x and is no longer supported...

It would be nice to improve this to include more information on where the property was originally consumed as well as where it was then mutated. Something like this would be nice to aim for:

Assertion Failed. You rendered "model.name" in "templates/example1.hbs" and modified it in "components/component-that-changes-name-on-init" in a single render. This was unreliable and slow in Ember 1.x and is no longer supported...

Here are some thoughts from @chancancode on how we might do this.

https://embercommunity.slack.com/archives/dev-ember/p1481392322000438:

screen shot 2016-12-11 at 16 34 45

Useful links:

@GavinJoyce
Copy link
Member Author

GavinJoyce commented Dec 11, 2016

For the You rendered "model.name" in "templates/example1.hbs" part, we should aim to include this data in meta when in debug mode so that we can use it when we detect a set on an already rendered property.

Looking at the available context where we currently set lastRenderedFrom[key], we don't appear to have easy access to anything that would allow us to identify the source template:

screen shot 2016-12-11 at 17 11 33

We have easy access to:

  • the parent object and meta
  • the reference
  • the transaction context, which includes:

screen shot 2016-12-11 at 17 21 23

It's not immediately clear to me how we can get any source template details from this. Some ideas the I'm going to explore:

  • Can we calculate the last DOM node rendered that has the reference? If so, we should be able to work out the containing view
  • As we're rendering the template, and at a higher level that within the reference, can we store additional data in a new lastRenderedIn meta key?

@GavinJoyce
Copy link
Member Author

GavinJoyce commented Dec 11, 2016

@GavinJoyce
Copy link
Member Author

GavinJoyce commented Dec 11, 2016

We have a reference when setting lastRenderedFrom. We also have easy access to the transaction context:

screen shot 2016-12-11 at 18 20 46

Somewhere within the context, we have an opcode which also uses the same reference:

screen shot 2016-12-11 at 18 16 33

This gives us access to bounds which includes pointers into the DOM. We can then determine which view this DOM belongs to and use this for the better assertion message.

Possible next steps:

  • how do we determine in the transaction context the last rendered opcode which uses the reference?
  • will this work for other cases, eg. helpers and nested references
  • can we map from DOM node to template identifier?

@chancancode
Copy link
Member

chancancode commented Dec 12, 2016

@GavinJoyce another idea: we can potentially use CurlyComponentManager#create and CurlyComponentManager#didRenderLayout as a pair and keep a stack of "current component" (we currently use that for the component instrumentation, so I think it could work). So at least we can tell you which component's template was causing issues (i.e. that should have enough information to report "...user.foo was first read while reading component:user-profile and later modified while rendering component:another-thing).

Probably need to add more instrumentation hooks to Glimmer if we want to get more granular than that.

@GavinJoyce
Copy link
Member Author

GavinJoyce commented Dec 13, 2016

@chancancode thanks, I've created a quick spike (😷 using a global to track the last rendered component 😷) and it works pretty well in our app:

Before:

You modified message.message_goal.description twice on <(subclass of Ember.Model):ember3486> in a single render. This was unreliable and slow in Ember 1.x and is no longer supported. See #13948 for more details.

After:

Assertion Failed: You rendered message.message_goal.description in component:message-edit-expanding-container-component and modified it in component:rules/predicate-date-value-component (<(subclass of Ember.Model):ember3486>) in a single render. This was unreliable and slow in Ember 1.x and is no longer supported. See #13948 for more details.

Next Steps:

@kiwiupover
Copy link
Contributor

Thanks heaps Mr Joyce. @GavinJoyce

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 a pull request may close this issue.

3 participants