Refined-github: Add delegated listener util, or switch from Sprint

[DrewML]

We've been manually creating delegated listeners for a bit now:

• https://github.com/sindresorhus/refined-github/blob/master/extension/content.js#L235
• https://github.com/sindresorhus/refined-github/blob/master/extension/content.js#L249
• https://github.com/sindresorhus/refined-github/blob/master/extension/content.js#L269
  -https://github.com/sindresorhus/refined-github/blob/master/extension/content.js#L235
• https://github.com/sindresorhus/refined-github/blob/master/extension/copy-file.js#L14
• https://github.com/sindresorhus/refined-github/blob/master/extension/reactions-avatars.js#L45

Rather than doing this manually everytime, it's probably worthwhile for us to abstract this.

I think we have 2 options:

1. Add our own util to handle this
2. Switch to a different DOM lib besides Sprint (Zepto is a popular one)

Thoughts?

[sindresorhus]

1. Let's just switch to a custom build of jQuery. See: https://github.com/sindresorhus/refined-github/pull/68#discussion_r55660573

[jamestalmage]

Sorry for the off topic question here, but are there no AJAX calls in this repo? (you're linked comment mentions -ajax). If so, that's impressive.

[sindresorhus]

@jamestalmage Yup, none. And even if we were to need it, we would just use the built-in Fetch API.

[jamestalmage]

jquerry 3 is out. Slim builds is a major new feature.

https://blog.jquery.com/2016/06/09/jquery-3-0-final-released/

[sindresorhus]

Nice. Official slim build. Let's do this! :)

[DrewML]

Sweet! I started working on this the other day with version 2, but the slim builds didn't provide much in the way of savings. I'll switch over to v3

[jamestalmage]

Still huge:

# Exclude everything but core:
grunt custom:-ajax,-css,-deprecated,-dimensions,-effects,-event,-offset,-wrap,-core/ready,-deferred,-sizzle

gzip dist/jquery.min.js

ls -hn dist
total 280
-rw-r--r--  1 501  20    83K Jun  9 22:14 jquery.js
-rw-r--r--  1 501  20    10K Jun  9 22:14 jquery.min.js.gz
-rw-r--r--  1 501  20    41K Jun  9 22:14 jquery.min.map

[DrewML]

I have this done, but.... I'm kind of doubting we really need a swiss army DOM lib at all. As far as I can tell, we're really only going to need it for delegate event listeners. Besides that, the majority of the functionality we rely on from Sprint/jQuery now exists as standards implemented in Chrome.

• $.fn.closest
• $.fn.find
• $.fn.not
• $.fn.hasClass, $.fn.addClass, $.fn.removeClass
• $.fn.each (for...of)

What are your thoughts on just pulling in a tiny lib for delegate listeners, and using standards for the rest of the DOM manipulation we rely on Sprint for? @sindresorhus @jamestalmage @paulmolluzzo

[sindresorhus]

Or just apply this tiny patch to Sprint: https://github.com/bendc/sprint/pull/20/files

[DrewML]

Doable. My main concern is that, at this point, something that half-supports the jQuery API feels like a leaky abstraction. I've personally been bitten by this on more than one occasion working on this project.

Examples:

• Incomplete API parity with .each
• Usage of .dom property to store collection
• size fn instead of .length prop (size was deprecated awhile ago in jQuery)

There are still other parts of jQuery's DOM API that someone may be tempted to use that won't be supported by Sprint. One example is attaching arbitrary data to a .on handler, but I'm sure there are more.

If we were to go with native DOM APIs, us + other contributors at least have a contract that the API we see documented is what we're getting.

[jamestalmage]

I agree - just do jquery or native. Personally, I don't feel jquery hurts you in an extension. It's not being downloaded on every visit. If they ever support mobile extensions, we will have to rethink that, but for now, jquery feels pretty well suited to the task.

[paulmolluzzo]

If we were to go with native DOM APIs, us + other contributors at least have a contract that the API we see documented is what we're getting.

I agree with the thinking here. By using the native API of the minimum supported version of Chrome it'll be pretty easy for everyone to find what can/can't be used.

Any custom-generated library is going to be a pain for newcomers to get used to ("Oh, you didn't include X?"). We'd have to document the custom build and maintain that documentation if/when there's a PR that suggests we update the asset. Also, to have all of jQuery when we don't need cross browser or legacy support seems heavy-handed.

If we go with native only we might want to add facades if we're reusing functions regularly to make it a little easier (example from refined-twitter), but for the most part I would think it's just a syntax change. We might get some verbose code in parts. For example, we're chaining .not().is().

---

@DrewML I wish you would have pressed y before pointing out those code examples in the OP. 😏

[DrewML]

@sindresorhus Your preference is still for jQuery over native APIs, right? If so I'll start moving forward on that

[sindresorhus]

I would prefer jQuery. For our use-case, there's very little perf benefit of using native APIs, but doing so creates more convoluted code.

[sindresorhus]

jQuery also fixes a myriad of browser bugs, yes even in the latest Chrome.

[hkdobrev]

Resolved in #252 🎉