#346 - add renderIfAttached method to View.js

[fyliu24]

add renderIfAttached method to View.js that only renders if the view is active and attached to the dom

[kentmw]

Why does the caller have to care about if it's attached? I'd rather just add this attached and active logic to the prerender method.

prerender: function() {
  return this.isAttached() && this.isActive();
}

The outward API shouldn't have to know or care if it's attached when calling render.

[kentmw]

Specifically, the view rendering should know if the content inside it will be affected if rendered detatched or deactivated. The view rendering should add it's own prerender logic to block a render command. I don't want to have to think about whether to use myChildView.render() or myChildView.renderIfAttached()

[fyliu24]

I think I'm convinced by you @kentmw. @mandragorn since you created the original issue? Do you have any argument that wants to do it this way instead of just a prerender logic?

[kentmw]

kent.willis
i'm just thinking this can be handled by a custom prerender method. I don't want to muddy the public render api
and i definitely don't want to think about whether to use render or renderIfAttached. If it's dangerous to render not attached, the view should know that and not render

josh.young
the issue is if the trigger factors into whether it should render or not
and I'm not sure that is the case
there was an issue where I had to add a render if attached to a view and the peer review comment was to add the ability to torso - that is what spawned the issue.

kent.willis
one of the baselines of the torso views is that calling render on it at anytime is fine
if that turns out to not be the case, i feel like it should be in the view itself

josh.young
yes, but we've already ran into issues where that is not the case. Shared view stealing being the biggest culprit right now.

kent.willis
yea, well that's poor implementation of shared views in tortso

josh.young
I honestly think render should never be called unless it is attached (questionable on whether it is active)
because you are just wasting resources if it isn't part of the dom
it is inefficient and provides no benefits (since render doesn't change state)
and whenever you call attach it renders anyway

kent.willis
this is obviously a bigger question

josh.young
🙂

kent.willis
your dom exists as dom fragments. your parents can guarentee that child dom exists regardless if it's attached

josh.young
I was hoping that would be sparked by the issue being created not by it being worked.
what is the benefit?
if it isn't shown and it will get rendered again when it gets attached, who cares?
I'm missing the benefit of having a view with a dom fragment that isn't part of the page.

kent.willis
first, i agree that re-rendering feels like a waste

josh.young
we don't do anything in torso to optimize and not render on attach.

kent.willis
but you walk shaky ground when you stop rendering

josh.young
why?
it doesn't change state except dom state

kent.willis
i know, hold on

josh.young
and your dom state is encoded in your view
hehe
ok

kent.willis
you're absolutely right, that detached re-rendering computes things that are never seen by the user. And that state isn't lost by not re-rendering. It comes down to whether the application cares about the reflection of true dom when it says to render. If you call render on a child view and it silently doesn't do anything, how are you supposed to know if the DOM inside it is correct or not. And if you don't allow views to render off the page, then you can't attach a child view to a parent view that hasn't been attached yet (no injection site). I believe this was one of the original motivations for rendering during initialization. Re-rendering off the page doesn't invoke a paint either, usually the most costly part of a render. In some circumstances isAttached can be just a costly as rerendering a dom fragment; it does an entire document search for the view. I think my opinion is that a view should be able to render itself off the dom but that we should look to limit those renders if it's not attached.

josh.young
so you are balancing the cost of isAttached vs. a render?

kent.willis
no, that's a tiny aside

josh.young
I don't understand the parent view comment I need an example or something

kent.willis
a large part is saying what happens when I say: this.myChildView.render();
you're saying that it could do nothing
i'm saying if your application says to render, you should be able to do: this.myChildView.$el.find('...') after it

josh.young
in what real world case would the .$el be used if it isn't being attached to the page?
right... is there a use case for that?
it feels contrived and text book, probably because I don't work with complicated applications.
I feel like if you are querying the dom you are better off querying the state of the child view instead.
since the dom is based on the state.
and if you want to do something to the child dom, then you are violating the fact that a view should only mess with its own dom/template.
I think practically I haven't seen a need to have the .$el exist except to attach it to the dom because the entire state of that dom should be reflected in the child view.
it breaks the separation of concerns (in my mind?)... I'm not trying to be difficult, but I really don't see a practical need for it.

kent.willis
here's the biggest usecase i see:
you totally remove Torso to be used as a node tool. Right now, you could (with maybe some small tweeks) use Torso views to generate the html or emails that a server might want to generate and pass to the client
it also means that I can't debug my application in the console

josh.young
see, I knew there was something I didn't think about.
well, that's not true
the debug bit

kent.willis
i have to attach it do the dom to understand what would be generated

josh.young
you just need to be better at debuggin 😛

kent.willis
care to share how if calling render() does nothing i see the $el

josh.young
again a usecase I haven't run across (debugging something that isn't attached to the dom)

kent.willis
really?

josh.young
nope
not once

kent.willis
weird

josh.young
when I'm debugging something it is because it is on the page and being rendered. Or not and that is an issue with the parent view or something.
but I've never needed to look at .$els directly.
all the state is in the view and I can use the template to render what it knows if I'm having trouble with .html, but then I see it because it is attached.
or I debug prepare
or a variety of other things, but I pretty much ignore the fact that the view has an .$el because it is generally attached.
the rendering in node bit I'll concede, though.

[Aside from Kent after reading this a second time: I often look at rendered dom fragments in a view to determine which view it is in the debugger. Often times it's a better natural identifier than cid or other fields. It will tell you which kind of view and the rendered content. As in a list view, etc.]

kent.willis
create a behavior that hooks into the prerender
do you guys use one view that uses a bunch of behaviors you guys like for all views?

josh.young
yea, I was going to ask before whether the result of prerender of a behavior is respected by the view.
we don't
because all of our views use different behaviors

kent.willis
we did over at RLS to generate guids on all views and adds hammer callbacks

josh.young
we haven't had that need yet.

... [conversation about hammer omitted] ...

kent.willis
yea. honestly, i see what you're saying, but i liked the assumption that render generates dom that is a reflection of its state. something separate from it's state in the application/dom. I could absolutely see a setting that says don't generate new dom if it's not on the document. Maybe it could just be a field on the view:

  Torso.View.extend({
      skipDetatchedRenders: true,
      skipDeactivatedRenders: true,
      ...
   })

i have fears of unknown consequences of changing an assumption baked into the lowest level. Does feedback or the myriad of things rely on it? maybe not

josh.young
sure as long as we can also trigger that directly (and leave render as the default). In the case of triggers.
so have a view where those are false, but still be able to trigger a render only if attached.
or something

[kentmw]

Ugh, I looked into adding:

Torso.View.extend({
    skipDetatchedRenders: true,
    skipDeactivatedRenders: true,
    ...
 })

But all views are detached before rendered and put back on. So, a view never renders where it is attached.

[kentmw]

For future dev, this is what I put in before realizing the issue:

if ((this.skipDeactivatedRenders && !this.isActive()) || (this.skipDetachedRenders && (!this.isAttachedToParent() || !this.isAttached()))) {
  this.trigger('render:skipped');
  return $.Deferred().resolve().promise();
}