Skip to content

Native view and jQuery-less History#2959

Closed
wyuenho wants to merge 28 commits into
jashkenas:masterfrom
wyuenho:native-view
Closed

Native view and jQuery-less History#2959
wyuenho wants to merge 28 commits into
jashkenas:masterfrom
wyuenho:native-view

Conversation

@wyuenho

@wyuenho wyuenho commented Jan 15, 2014

Copy link
Copy Markdown
Contributor

This PR improves on PR #2865:

  • Adds a very small polyfill of addEventListener and removeEventListener for IE8.
  • Slightly faster, a much more compact and more compatible matchesSelector implementation
  • Fixes a bug with remove where PR#2865 confuses removal with detachment. $.fn.remove will remove event handlers while PR#2865 does not. Failure to do so will make some event handlers depending on this behavior fail. This broke Backgrid.
  • A native setElement that's more faithful to jQuery's behavior. The code is improved from Backbone.native. Specifically, you can supply an HTML snippet, a CSS selector or a DOM element, just like jQuery.
  • No more useNative flag and branching. The old View had been extracted into BaseView and _ensureElement, setElement, delegateEvents and undelegateEvents uses native APIs. So Backbone will have a baseline View that's fast by default.
  • BaseView#$ just delegates to this.el.querySelectorAll and returns a node list.
  • Removes View#find and View#findAll. They add no value and it's not even the correct implementation of the proposed Selectors API Level 2 spec.
  • Introduces _delegateEvents and _undelegateEvents for subclasses to override.
  • Much much more compact implementation of BaseView#_delegateEvent and BaseView#_undelegateEvents in place of util.delegate and util.undelegate. BaseView#_delegateEvent will also work on IE8.
  • Removes the superfluous jQuery shim that's utils. This PR has effectively removes hard dependency on jQuery already by pulling out a BaseView that's basically the old view + 2 hooks and all native DOM API calls. The only remaining dep on $.ajax can be overcome easily by replacing Backbone.ajax with one of the many $.ajax alternative implementations out there.
  • The new Backbone.View is a 26 line extremely light weight subclass of Backbone.BaseView that's 100% backward compatible with the old jQuery-based View.
  • Comments everywhere and better tests.
  • A few less LOC in the Router than Remove jQuery dependency. #2865 due to the event listener polyfills.

Tests passed on:

  • IE 8+
  • Chrome
  • Firefox
  • Safari
  • Opera

Performance:

http://jsperf.com/backbone-patch-22be8f9/2

Around 70% faster than the jQuery-based View and slightly faster than #2865, by a couple of percentages consistently.

Remaining issues:

  • Should BaseView be called something else? Should the native View subclass a refactored jQuery-based View instead? Code size will be exactly the same.
  • IE 8 support in BaseView? See options here.

cc. @paulmillr @akre54

Comment thread backbone.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like if we're going to have a standards-based BaseView and a regular jQuery View that BaseView should be as slim as possible and only care about the latest browsers / modern web standards, rather than selectively implementing polyfills for some browsers (IE8) but not others (IE7) - leave jQuery for people who care about old browsers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally I'd agree but that's just 15 lines of extra code. I'd pay an extra 300 bytes for compatibility. That and many of my clients using Backgrid are still on their network's IE8. The jQuery based View is already slow on most modern browsers. It's unbearable on IE8.

@wyuenho

wyuenho commented Jan 16, 2014

Copy link
Copy Markdown
Contributor Author

Ok npm test weirdness with PhantomJS and CommonJS fixed.

@wyuenho

wyuenho commented Jan 16, 2014

Copy link
Copy Markdown
Contributor Author

I think the remaining 2 issues are, do we want compatibility with IE8 in BaseView? And are the BaseView/View naming and arrangement OK?

@jashkenas

Copy link
Copy Markdown
Owner

@wyuenho What exactly is the question about polyfills?

@wyuenho

wyuenho commented Jan 16, 2014

Copy link
Copy Markdown
Contributor Author

@philfreo said the addEventListener/removeEventListener/matchesSelector shims shouldn't be in Backbone because people who care could just use the jQuery backed View.

@paulmillr Wants the private matchesSelector polyfill exposed and reachable.

I just want to know whether you think it's worthwhile to keep the polyfills first, if not, lots of problems become moot.

@jashkenas

Copy link
Copy Markdown
Owner

said the addEventListener/removeEventListener/matchesSelector shims shouldn't be in Backbone because people who care could just use the jQuery backed View.

That's probably true.

Wants the private matchesSelector polyfill exposed and reachable.

I don't think so. We should keep the internal workings private and hidden.

@wyuenho

wyuenho commented Jan 16, 2014

Copy link
Copy Markdown
Contributor Author

Yep. I don't think it's Backbone's jobs to provide polyfills either. Should I remove the polyfills and compat code for IE8 in BaseView then?

@jashkenas

Copy link
Copy Markdown
Owner

If you're going to use jQuery for backwards compatibility, then yes.

@wyuenho

wyuenho commented Jan 16, 2014

Copy link
Copy Markdown
Contributor Author

Ok IE8 compat code gone.

@wyuenho

wyuenho commented Jan 16, 2014

Copy link
Copy Markdown
Contributor Author

Are you fine with the BaseView and View arrangement?

@wyuenho

wyuenho commented Jan 17, 2014

Copy link
Copy Markdown
Contributor Author

Ahhh actually @jashkenas I think I'm gonna put the IE8 compat code back because I just realized the router is using jQuery too. It's totally unnecessary and removing that dependency while preserving compatibility will have to use the addEventListener/removeEventListener shims. The compat code really isn't that big.

@wyuenho

wyuenho commented Jan 17, 2014

Copy link
Copy Markdown
Contributor Author

Ok the Router is now rid of jQuery dependency. Tests still pass on all modern browsers and IE8+

Comment thread backbone.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to not necessitate this if check upon every invocation? Small optimization but should be a simple one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well if you want to go after absolute speed then yes I can optimize not only the if statement but the entire extra function call away.

But since IE and Safari's window.addEventListener != Element.prototype.addEventListener, I'll have to litter 4 more if statements inside the Router to preserve IE8 compatibility.

If we decide to drop IE8 support (which I think is reasonable), then I can remove the event listener shims altogether. In fact, we could probably drop the jQuery-backed View as well if that's the case. Still waiting for the word on this one from other Backbone core devs.

cc. @braddunbar @caseywebdev @philfreo @tbranyen @tgriesser @akre54 @wookiehangover @gsamokovarov @jashkenas

@wyuenho

wyuenho commented Feb 16, 2014

Copy link
Copy Markdown
Contributor Author

I'm not sure it requires introducing their own base support.

Do you have a better idea that doesn't regress back to something like #2865?

@ghost

ghost commented Feb 16, 2014

Copy link
Copy Markdown

Do you have a better idea

Have Backbone expose API that can be hooked into by adapters instead of funneling everything through Backbone.$ (things like your _delegateEvents allow more spots for adapters to tap into). You can see this being done with CanJS and their adapters for Dojo, jQuery, MooTools, YUI, & Zepto. This would create a buffer between Backbone and the DOM lib to smooth over API incompatibilities or introduce optimizations.

It's similar to what you're doing in BaseView except would be separate of Backbone.js.

@wyuenho

wyuenho commented Feb 16, 2014

Copy link
Copy Markdown
Contributor Author

Which is basically #2865's utils. Backbone.$ is already an adaptor. All you have to do is implement the list of jQuery interfaces @braddunbar said and delegate to whatever you want. Ultimately this still won't solve the problem of needing a mix of fast BaseView and more compatible but slower jQuery backed View. Going through an extra layer of function calls also won't make your Views nearly as fast as BaseView. Backbone has always known to be fast, except when you have lots of Views. This PR improves that situation.

The major contention remaining seems to be IE8 support in BaseView. The way out depends on how far you want to go to isolate jQuery dependency in Backbone so that it becomes realistic to swap it out entirely (using jQuery in Hisitory is completely unnecessary). Here are the options:

  1. Rollback the changes in History so that those 13 lines of *EventListener and 7 lines of matchesSelector shims can be removed. Users who need to support IE8 with BaseView can drop in their own DOM EventTarget and Element.matches shims. There are a bunch out there for EventTarget, but AFAIK, there's only 1 satisfactory matchesSelector shim. It's non-trivial to source a good one or write a matchesSelector shim properly. This PR provides a private, minimalist (thanks to underscore) but both forward and backward compatible implementation for both.
  2. Same as 1, but keep the matchesSelector shim for IE8. You still have to drop in an EventTarget shim to use BaseView with IE8, or you can just override BaseView.prototype._[un]delegateEvents to add a call to attach|detachEvent.
  3. Keep the changes in History, but remove 13 lines of *EventListener shims. This will break IE and Safari, but you can drop in an EventTarget shims.
  4. Same as 3 but drops 4 lines of element*EventListener shims.

All options are either asking too much or halfway there but not enough. After writing BaseView, removing jQuery dependency in History from inside Backbone becomes trivial, so I've decided this PR is the best compromise I can come up with. It's quite minimalist for what it does. Hopefully this clears things up.

@ghost

ghost commented Feb 16, 2014

Copy link
Copy Markdown

Which is basically #2865's utils. Backbone.$ is already an adaptor

I figured you would say that, which is why I snipped your quote in my reply :)

Ultimately this still won't solve the problem of needing a mix of fast BaseViewand more compatible but slower jQuery backed View.

Sure it can. If there's something preventing it, expose an API point.

Going through an extra layer of function calls also won't make your Views nearly as fast as BaseView.

Talking about performance before there's an implementation is a bit premature.

All options are either asking too much or halfway there but not enough.

I think you're missing my greater point. I've shown, using CanJS as an example, that adapters can be made to address a variety of issues and don't necessarily belong in core. Nothing is preventing your BaseView from being implemented as an external adapter. If there is a block, because of some missing API, then core should ensure to expose it. That's it.

This would keep Backbone minimal, without having to get into the DOM realm, and still allow for 3rd-party adapters to address a variety of needs.

@wyuenho

wyuenho commented Feb 16, 2014

Copy link
Copy Markdown
Contributor Author

@Dw40 Talk is cheap. Talking about theoretical possibilities without concrete implementations is even cheaper. Show me some code that doesn't look like #2865 or Backbone.Native or Backgrid.View, which is basically a rewrite of View, that is equally fast as BaseView.

@ghost

ghost commented Feb 16, 2014

Copy link
Copy Markdown

Talk is cheap. Talking about theoretical possibilities without concrete implementations is even cheaper

I've pointed to another real MV* framework using adapters successfully. There's your concrete real world implementation 😀. The specifics of how one does it is less interesting, the important thing is ensuring Backbone doesn't throw up roadblocks preventing it from being done.

Show me some code that doesn't look like #2865 or Backbone.Native or Backgrid.View, which is basically a rewrite of View, that is equally fast as BaseView.

It's not my or Backbone's responsibility to create another DOM adapter. Backbone should just make sure it isn't blocking that from happening. Developers have different browser compat needs and I don't think its Backbone's concern to address.

@braddunbar @akre54 I think this PR should be closed and, if anything, the effort refocused on ensuring Backbone isn't blocking native barebone adapters from being made.

@wyuenho

wyuenho commented Feb 17, 2014

Copy link
Copy Markdown
Contributor Author

If you can't back up your claim that having an extra adaptor can satisfy the requirements as well as this PR, or contribute to a discussion constructively by offering feasible suggestions, please hold your peace.

Backbone doesn't need another adapter, Backbone already has Backbone.$. Which doesn't help much in terms of isolating jQuery dependency and coexistences of fast BaseView and jQuery backed Views within the same app. Just look at the size of Exoskeleton and Backbone.Native.

@ghost

ghost commented Feb 17, 2014

Copy link
Copy Markdown

If you can't back up your claim that having an extra adaptor can satisfy the requirements as well as this PR, or contribute to a discussion constructively by offering feasible suggestions. Please hold your peace.

I've been offering feasible suggestions and constructive feeback, despite your attitude, but you've chosen to ignore it.

Which doesn't help much in terms of isolating jQuery dependency and coexistences of fast BaseView and jQuery backed Views within the same app.

You've mentioned this a couple of times. I'm all for making it easier to swap out jQuery but again I don't think it requires Backbone taking on BaseView and getting into the DOM any more than it has to.

Just look at the size of Exoskeleton and Backbone.Native.

Backbone.Native handles ajax, event namespaces, and a few other things not addressed by your BaseView and is still only ~1.5kb. I think that's reasonable and strengthens the case for not needing this PR.

@wyuenho

wyuenho commented Feb 17, 2014

Copy link
Copy Markdown
Contributor Author

@Dw40 How about you answer this question directly? as your main objection is (was?) IE8 support in BaseView and back up YOUR claim here? My attitude towards you specifically is well justified because you:

  1. First demonstrated a complete lack of understanding of the intentions of this PR
  2. Then you continued to mischaracterize the intentions by ignoring of how this PR does NOT affect the people who need the jQuery backed View.
  3. Then you continued to interrupt discussion by dragging on about unrelated matters like AJAX.
  4. Then you made baseless accusations.
  5. Then you continued to pick on things that were never even a goal.
  6. Then continued to make useless suggestions that have long been considered by many and rejected for needlessly increasing code size for 0 benefit.
  7. Then you avoided to offer an answer to a technical choice.
  8. Then you refused to backup your own bold claim.
  9. Then you edited away your snarky useless responses, I didn't quote.

All this trollish behavior makes me very suspicious of your motives, especially when coming from someone who has a suspiciously recently opened Github account, who have 0 contribution to open source and someone who is obviously not a newcomer to Web development.

Why are you so hell bent on derailing this PR? You obviously have no use for this PR, but is there anything specific that actually majorly affects you negatively? You may not need this, other people do. Now take your complain about every other feature you don't need in every other library that you've used somewhere else.

I don't think it requires Backbone taking on BaseView and getting into the DOM any more than it has to.

Well, show me code you would use to isolate jQuery from the View in your own application code without rewriting Backbone.View. I'm all out of ideas.

There will be no more reply from me to this thread. I've laid out every technical decision I've considered and presented the case why this PR is the best I can come up with to solve the problem of further isolating jQuery dependency and introducing a really fast BaseView that can coexist with the jQuery backed View while introducing minimal code.

If @jashkenas chooses an option from this list, I'll go make my amendments, otherwise take it or leave it.

@ghost

ghost commented Feb 17, 2014

Copy link
Copy Markdown

How about you answer this question directly?

I have answered it.

as your main objection is (were?) IE8 support in BaseView

Yes, among other things. I don't think Backbone needs to get into the DOM, or its compat, any more than it has to. You brought up Backbone.Native, which looks a lot like BaseView, and shows it can be done outside of Backbone core.

My attitude towards you specifically is well justified because

You've thrown a lot of stones there.

All this trollish behavior

I think you're mixing up trollish behavior with those like @braddunbar, @tbranyen, and myself who respectfully disagree with the PR and think it isn't needed in core. I've been replying to make that case.

You may not need this, other people do.

I disagree when there are options like Backbone.Native. It's clear your goal, removing the dependency on jQuery, opting for native DOM API, and improving performance, can be achieved without changes to Backbone.

@paulmillr

Copy link
Copy Markdown
Contributor

Jimmy, please stop arguing with this dude. I don't think his opinion is relevant. What matters to us, is Jeremy's, or maybe other maintainers thoughts.

@akre54

akre54 commented Feb 17, 2014

Copy link
Copy Markdown
Collaborator

nah, the problems with jQuery-compliant API "adapters" are twofold:

  1. creating a new context is costly, and causes the large speed decreases @paulmillr and @wyuenho have pointed out
  2. if you're only implementing a subset of jQuery's API to appease Backbone, you quickly run up against the law of leaky abstractions, where most of the features you need are hidden behind your thin abstraction and require ugly workarounds.

It's much more efficient and logical for Backbone to expose its DOM manipulation API as a set of pluggable options that can be overridden on the fly (a la earlier versions of #2865). I not a fan of the separate View/BaseView classes, and I think #2865 should have been more than sufficient (minus useNative) for most people. While @wyuenho has a valid argument of a separate speedy view, personally I'm in favor of leaving that to a plugin (like BackGrid is currently) and making Backbone.View more flexible about working with speedier native methods.

@wyuenho

wyuenho commented Feb 17, 2014

Copy link
Copy Markdown
Contributor Author

@akre54 How do you suppose we could preserve backward compatibility with $el in place by layering yet another adaptor?

@akre54

akre54 commented Feb 17, 2014

Copy link
Copy Markdown
Collaborator

$el is only if you have a jquery analog. Otherwise theres just el.

My only question is about view.$
On Feb 17, 2014 1:18 PM, "Jimmy Yuen Ho Wong" notifications@github.com
wrote:

@akre54 https://github.com/akre54 How do you suppose we could preserve
backward compatibility with $el in place by layering yet another layer of
adaptor?

Reply to this email directly or view it on GitHubhttps://github.com//pull/2959#issuecomment-35309487
.

@wyuenho

wyuenho commented Feb 17, 2014

Copy link
Copy Markdown
Contributor Author

So now all of a sudden the existence of $el can't be relied on by plugin authors anymore because they'll never really know which adaptor people are using. So instead of having 2 known variables of BaseView, and View, people now have to deal with N possibilities and a whole bunch of mutually incompatible "plugins" because hey now you have jQuery and MooTools and YUI all inside a single app because of the plugins that you use. No wonder CanJS never picked up.

@akre54

akre54 commented Feb 17, 2014

Copy link
Copy Markdown
Collaborator

But they cant rely on $el from base view to have a jquery-compatible interface anyway.

Best just to rely on el, which all views will have, or check for $el or wrap in Backbone.$ if absolutely necessary.

@wyuenho

wyuenho commented Feb 17, 2014

Copy link
Copy Markdown
Contributor Author

There's no this.$el in BaseView, just this.el. So you need to use the DOM API inside subclasses or Backbone.$(this.el).

I don't understand why BaseView can't be the baseline View implementation, which acts like an adaptor anyway, and have every other alternative implementation that uses a different DOM lib inherit from it. As the new Backbone.View has shown, it's trivial.

@akre54

akre54 commented Feb 17, 2014

Copy link
Copy Markdown
Collaborator

Because in my mind, either you're going jquery or you're going native. Any other adapter you're using a Backbone.$ analogue anyway.

If you want them both on the same page, that screams plugin to me.

@wyuenho

wyuenho commented Feb 18, 2014

Copy link
Copy Markdown
Contributor Author

All plugins have to start somewhere. It's not possible to extend View to use another DOM lib or the DOM API without a complete rewrite of View. This PR provides a BaseView that implements the basic View semantics, it just happened to use the DOM API by default.

You may start out native only or jQuery only, but eventually you'll have to mix in both if you want to speed things up or have greater compatibility. You don't see that now only because it's not possible now.

This PR surpasses #2865, which was closed by the way, in terms of flexibility due to its granularity, while introducing a lot less code. What's not to like about that? This PR supersedes #2865.

This continuing theme of binary all-or-nothing mode of thought is very perplexing.

@wyuenho

wyuenho commented Feb 18, 2014

Copy link
Copy Markdown
Contributor Author

Just curious, @akre54, would this PR taste better if I introduced BaseView#_removeElement and bring back the previous View#make to BaseView?

@akre54

akre54 commented Feb 18, 2014

Copy link
Copy Markdown
Collaborator

if I introduced BaseView#_removeElement and bring back the previous View#make

Yeah that sounds reasonable. If our focus is on removing jQuery from View specifically, why not have hooks in View for things like delegate/undelegate (for a single event, called by delegateEvents/undelegateEvents), and then make will get called by _ensureElement like it used to. That way, NativeView is a plugin, a subclass of View, minus the jQuery-specific stuff.

@acstll acstll mentioned this pull request Feb 19, 2014
@wyuenho

wyuenho commented Feb 21, 2014

Copy link
Copy Markdown
Contributor Author

Closing in favor of #3003 and #3006.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.