germanysbestkeptsecret / Wookmark-jQuery

A jQuery plugin to create a dynamic, multi-column layout.
MIT License
2.64k stars 759 forks source link

Use own $ variable, make sure Jquery can be loaded #228

Closed falconshark closed 7 years ago

falconshark commented 7 years ago

If use $ variable directly, Wookmark may be cannot call Jquery function (Such as running on Drupal).

yookoala commented 7 years ago

I have same issue using the library with Drupal. Drupal use jQuery with noConflict mode to avoid conflicting use of the $ variable. Therefore jQuery plugin that uses $ alias directly will have problem.

As changed by this PR, it is a common practice to use immediately-invoked function expression to protect $ alias in your scope. It is also cited in the official learning doc.

Hope this PR is accepted.

Sebobo commented 7 years ago

Hey, thanks for the PR! Of course this has to be fixed, I overlooked that because I test on the existence of jQuery first. But I'm not sure if the closure is the best idea. Then the plugin looks more like a jQuery plugin which it actually isn't necessarily. Instead the ~3 places where jQuery is used could just be changed to use jQuery instead of $.

What do you guys think?

yookoala commented 7 years ago

Immediate-invoked function expression binds the jQuery variable when the script is called. Whereas if you changed all $ to jQuery, the script will simply call the current window.jQuery object.

In that sense, immediate-invoked function is much more friendly to jQuery users when they need to use multiple jQuery versions together. They simply have to arrange the script call order properly like this:

...
<script src="js/jquery-3.1.1.js" />
<script src="js/Wookmark.js" />
<script src="js/jquery-1.17.1.min.js" />
<script src="js/some-old-plugin.js" />
...

So as long as Wookmark is depending on jQuery, I think adopting the pattern is a better idea than replacing all $ with jQuery.

Sebobo commented 7 years ago

@yookoala true, I never do that, but I know there are websites which need it. I'll try to make a new release in the next days.

falconshark commented 7 years ago

@yookoala @Sebobo Thank you very much! :)