ngryman / jquery.finger

:v: jQuery touch & gestures, fingers in the nose.
https://ngryman.sh/jquery.finger/
MIT License
423 stars 66 forks source link

Add AMD and CommonJS definition #34

Closed jacobbuck closed 9 years ago

jacobbuck commented 9 years ago

@ngryman no love for AMD or CommonJS? :broken_heart:

ngryman commented 9 years ago

Oh sorry forgot to answer...

Well I don't know about this one. finger is a jquery plugin. Using jquery as a cjs/amd dependency feels bad to me, as you need explicit requires in each module of your app / plugins, which is kind of counter productive. Keeping it as a global should be better imho.

jacobbuck commented 9 years ago

All good.

For our use case we're loading jQuery via AMD, and jQuery has noConflict defined, so wrapping it in a define makes this plugin work for us.

My pull-request does return Finger, so you can still interact with it directly:

define(['jquery', 'jquery.finger'], function ($, Finger) {
  Finger.preventDefault = true
  $('a').on('tap', function () {...});
});

Also I've based it off the UMD jqueryPluginCommonjs pattern (see: https://github.com/umdjs/umd/blob/master/jqueryPluginCommonjs.js) which doesn't return anything by default.

ngryman commented 9 years ago

So you're using jquery with amd because 2 versions of it live in the same page if I understand it right? Just curious about the use case itself.

Anyway, after some thought I will accept your PR because it probably exists use cases where users don't have another choice than requiring jquery explicitly. Even if I still feel that this is not the right way to do, I don't want to discriminate them.

Could you just squash your 2 commits?

Thanks !

jacobbuck commented 9 years ago

Cool, and thank you :blush:

The main use case for us is taking advantage of async bundle loading with require.js, and by having our plugins dependant of jQuery with AMD means the required plugins load and execute at the right time before our scripts.

In the case of having 2 versions of jQuery, we do also have some legacy pages with javascript that has an older modified version of jQuery in the global scope (horrible I know), so using $.noConflict is good for avoiding conflicts with that.

Will squash those commits now.

ngryman commented 9 years ago

Thanks :sunglasses:

ngryman commented 9 years ago

FYI, released as 0.1.3.

jacobbuck commented 9 years ago

:+1: