akre54 / Backbone.NativeView

A reference implementation of a native Backbone.View
MIT License
113 stars 18 forks source link

events issues when no el is set #23

Open designermonkey opened 9 years ago

designermonkey commented 9 years ago

I'm not sure if this is NativeView or Backbone itself, so apologies in advance.

When defining an element that is in page el: '#some-element if the element doesn't exist, it is rightly set to null.

In the render function I can return early in this case, but any events that are defined are attached to the document as the element isn't there. This is causing issues and interruptions for the rest of the page.

What should I be doing in this case? I've tried using undelegateEvents to no effect, as they haven't been correctly delegated, and the code looks for this.el.

akre54 commented 9 years ago

el is simply set to the result of document.querySelector, which for a non-existent el will be null. (I'm sure you've already gotten that far, just getting it on the record).

jQuery is known for being newbie-friendly and failing gracefully, but I'm not sure that's the tack we should take. It often leads to silently swallowed errors and overly permissive code.

$('foo')
> []
$('foo').on('bar', () => { throw new Error })
> []
$('foo').trigger('bar')
> []

Our two options as I see it are:

  1. Keep this in userland; you're required to give an existing element or selector.
  2. NativeView gives the view a dummy element to no-op bind against like this:
this.el = document.querySelector(element) || document.createElement('div');

It's a little wasteful but not especially harmful. May be unexpected though?

What's your use case btw? When would you try to pass an el to a view that isn't already in the DOM?

designermonkey commented 9 years ago

The use case is me being lazy and using a specific module on every page whether the element is there or not.

The way I got round it so far was to make events a function and check for el in there.

It's been a while since I wrote JS as plain DOM. I can't remember, why is it that the event is attached to the document? Is that bubbling at work there?

IMO, the code that attaches the event should check for the el or the selector in the event string (if defined) and if it doesn't exist, shouldn't attach events. I know this is more like jQuery behaviour, but it would prevent unexpected results.

Having the element as null is actually quite useful and allows a short circuit of sorts, so we can prevent a module from functioning.

akre54 commented 9 years ago

We could do that. delegate could short-circuit if this.el is null. We already guard for an falsey element in undelegate and undelegateEvents.

Wanna open a pull with some tests for missing elements? Let's make sure nothing else would break with this.

I can't remember, why is it that the event is attached to the document? Is that bubbling at work there?

What part are you referring to? This section of delegate? That's so we can write {"click .foo": "handleFooClick"} and be able to lazily add a ".foo" element after delegation.