Introduce a "ready" event for views

[paulfalgout]

Description

Currently the best place to assume a view is ready to do most things is onRender (with the exception of needing to know the view is in the DOM)

This is all fine and dandy until you initialize a view with rendered DOM. Then when the view is created it is already rendered, and therefore does not need to be rendered and thus onRender would never occur. The current solution is awkward and asks the user to either use "template: false" or re-render the view.. Or in some cases setup view related things inside initialize and then potentially later in onRender as well.

We can simplify this by throwing a ready event once we know the view is rendered, regardless of where it was rendered.

It also way simplifies understanding where to do things.

It doesn't necessarily solve attachment, as a similar problem comes up if the view is already attached.. but I think that's more edge case-y and can be handled by:

onReady() {
  if (this.isAttached()) {
    this.showJQueryPlugin()
  } else {
    this.on('attach', this.showJQueryPlugin);
  }
}

[rafde]

I am guessing domReady isn't enough?

[paulfalgout]

I don't really know how you mean. I guess I am looking for one event that would work for setting up children for a view that is prepopulated, but has a template for rerendering.

[rafde]

sorry, I mean domRefresh.

The suggestion of an ready event seems like it's strictly for the case that of a view is already in the DOM. so I figure dom refresh is good for that case.

But if people need an event strictly to know that the view is ready since the target element is a pre-rendered dom element, then I guess this is good. Would having this event mean not having to trigger other events like before:render and render, since the view's element is already rendered on the page? Or not triggering before:attach and attach. Cutting these events down for this case could help with perf.

But are there past issues related to this being an issue?
Maybe some community 👍 would help figure a need for this.

[paulfalgout]

domRefresh only triggers when a view is attached. I think this event should not care about attachment.

I actually see the need in the community for this frequently: #3128 (comment) And I suspect as we better support pre-generated html, we will run into this issue more often.

The crux of the issue is:

• We tell people the best place to show childview is onRender
• If you initialize a view with an existing el, you will not get render events (or attach events assuming the el is already attached to the DOM)
• So for views with an existing el most people use template: false, and then force a render event even though the view isn't rendering anything. But this doesn't work if you want the view to take over rendering after the initialization with an existing el. It also feels like a hack.
• So now you're left with creating a view that potentially sets up children etc during initialization if isRendered() and then also in onRender

I think it'd be better to give the users a single event, which by name says, "now you can do stuff"

Honestly I think it could replace dom:refresh. Dom refresh is for the view attach and any render after, but ready would be any time the view is ready.. sure it may not be in the DOM, but then just use this.isAttached() inside your onReady for attach specific stuff.

[paulfalgout]

A code example of how this would help:

const MyView = Mn.View.extend({
  regions: {
    'fooRegion': '.foo-selector'
  },
  initialize() {
    if (this.isRendered()) {
      // onRender won't fire if the view is already rendered on init 
      this.triggerMethod('ready');
    }
  },
  onRender() {
    // If this view is re-rendered, we will want to setup the children again
    this.triggerMethod('ready');
  },
  onReady() {
    const myChildView = new MyChildView({ el: this.$('.bar-selector')[0] });
    this.showChildView('fooRegion', myChildView);
  },
  template() _.template('<h1>Hello World!</h1><div class="foo-selector"></div>'),
  modelEvents: {
    'change': 'render'
  }
});

const $existingEl = $('.some-selector');  // pre-rendered by the server

myApp.showView(new MyView({ el: $existingEl[0] });

And maybe the better solution is to throw the related events for when isRendered and isAttached are set.. but those functions aren't called... and it's odd that you'd get a before:render in those cases.. but we can't necessarily leave them out either..

[JSteunou]

I understand the logic but I'm in favor of using render event for this case. I remember from 0.9 to 1.x then to 2.x I went from onShow then onDOM... then onAttach then onRender... hard to keep up with all those changes just to have the right moment in the life cycle of the view where you can manipulate DOM.

I think everyone get used to use onRender now (at least in my team), it's quite a good name, I think it is the best moment in view life cycle and it became a good practice. Please do not add a new name, new practice.

If someone uses a view with any exiting element, then he knows its view before:render has no meaning, if really we cannot hide this event.

[paulfalgout]

The moving of one event name to another is precisely why I'm suggesting we abstract it. We've been trying to find and constantly modify what event is best to consider the view "ready" to act on.. and as things have changed in the code, the best practices have changed as well. And now as we're getting new use-cases (server rendered HTML) we're realizing that onRender doesn't really work either..

So I think there's 3 options:

• Regardless of if there's any rendering happening trigger a "render" event for pre-existing DOM. (This feels hacky to me.. in this case.. render would not necessarily represent that the model data went through the template)
• Tell people there is no render event for pre-existing DOM so do something like the example above (This feels like pre-existing DOM is not a concern)
• Add a new event that isn't tied to what Marionette is doing, that can mean the view is usable, regardless of how Marionette may change in the future. (This feels like we're constantly moving what event to use.. however perhaps for the last time)

[JSteunou]

I understand but could seems like https://xkcd.com/927/ from Marionette user PoV.

[blikblum]

I would follow the motto "Make the common things easy, rare things possible"

Given that is possible, and relatively easy, to handle use cases with current primitives, like the presented above, i would avoid introducing a new event and document the usages as recipes.

[paulfalgout]

Perhaps Mn is not "ready" for this 🤓

[paulfalgout]

Closing for #3128

[paulfalgout]

note to self: if we ever implement this a Marionette.View that is template:false should be "ready" after initialization

[blikblum]

I changed my opinion about it and i think is a good feature

[paulfalgout]

While doing some region work, I definitely think it's time to readdress this

[paulfalgout]

It's possible this should just wait for v5 since the behavior will be moderately different for nondestructive views.. But we could introduce it, documenting the upcoming breaking change, so that people can migrate to it now and reap the rewards come v5.