From 5b41964b7b56af10c146bf60a77826f68969f197 Mon Sep 17 00:00:00 2001 From: dev101 Date: Wed, 20 Jan 2016 21:25:49 +0300 Subject: [PATCH 1/3] Calling this[fn].bind required extra check It is not valid to assume that Sortable's **prototype contains what you expect it to**. In my environment, Sortable is loaded by `requirejs`, and the **prototype is modified** in requirejs's loader callback by injecting a special `__moduleId__` property into the prototype, in a fashion like this: ```javascript requirejs.onResourceLoad = function(context, map, depArray) { context.defined[map.id].prototype.__moduleId__ = map.id; }; ``` At [Sortable.js#L232](https://github.com/RubaXa/Sortable/blob/master/Sortable.js#L232) this effectively leads to trying to call `.bind` on a `String` object, resulting in `this[fn].bind is not a function` error. There should be a smarter way to copy functions from the prototype, so far I can suggest an extra check for object type before calling `.bind`: ```javascript if (fn.charAt(0) === '_' && typeof(this[fn].bind) === 'function') { this[fn] = this[fn].bind(this); } ``` --- Sortable.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sortable.js b/Sortable.js index d271999e8..164de9677 100644 --- a/Sortable.js +++ b/Sortable.js @@ -228,7 +228,7 @@ // Bind all private methods for (var fn in this) { - if (fn.charAt(0) === '_') { + if (fn.charAt(0) === '_' && typeof(this[fn].bind) === 'function') { this[fn] = this[fn].bind(this); } } From c28906530538c3f67170e229b4da5209dd9e204e Mon Sep 17 00:00:00 2001 From: dev101 Date: Fri, 22 Jan 2016 19:25:05 +0300 Subject: [PATCH 2/3] Module definition improved --- knockout-sortable.js | 65 +++++++++----------------------------------- 1 file changed, 13 insertions(+), 52 deletions(-) diff --git a/knockout-sortable.js b/knockout-sortable.js index dcb470a83..181a42b9b 100644 --- a/knockout-sortable.js +++ b/knockout-sortable.js @@ -1,56 +1,17 @@ -(function (factory) { - "use strict"; - //get ko ref via global or require - var koRef; - if (typeof ko !== 'undefined') { - //global ref already defined - koRef = ko; - } - else if (typeof require === 'function' && typeof exports === 'object' && typeof module === 'object') { - //commonjs / node.js - koRef = require('knockout'); - } - //get sortable ref via global or require - var sortableRef; - if (typeof Sortable !== 'undefined') { - //global ref already defined - sortableRef = Sortable; - } - else if (typeof require === 'function' && typeof exports === 'object' && typeof module === 'object') { - //commonjs / node.js - sortableRef = require('sortablejs'); - } - //use references if we found them - if (koRef !== undefined && sortableRef !== undefined) { - factory(koRef, sortableRef); - } - //if both references aren't found yet, get via AMD if available - else if (typeof define === 'function' && define.amd){ - //we may have a reference to only 1, or none - if (koRef !== undefined && sortableRef === undefined) { - define(['./Sortable'], function(amdSortableRef){ factory(koRef, amdSortableRef); }); - } - else if (koRef === undefined && sortableRef !== undefined) { - define(['knockout'], function(amdKnockout){ factory(amdKnockout, sortableRef); }); - } - else if (koRef === undefined && sortableRef === undefined) { - define(['knockout', './Sortable'], factory); - } - } - //no more routes to get references - else { - //report specific error - if (koRef !== undefined && sortableRef === undefined) { - throw new Error('knockout-sortable could not get reference to Sortable'); - } - else if (koRef === undefined && sortableRef !== undefined) { - throw new Error('knockout-sortable could not get reference to Knockout'); - } - else if (koRef === undefined && sortableRef === undefined) { - throw new Error('knockout-sortable could not get reference to Knockout or Sortable'); - } +(function (root, factory) { + if (typeof define === 'function' && define.amd) { + // AMD. Register as an anonymous module. + define(['knockout', 'Sortable'], factory); + } else if (typeof module === 'object' && module.exports) { + // Node. Does not work with strict CommonJS, but + // only CommonJS-like environments that support module.exports, + // like Node. + module.exports = factory(require('knockout'), require('sortablejs')); + } else { + // Browser globals (root is window) + root.returnExports = factory(root.ko, root.Sortable); } -})(function (ko, Sortable) { +})(this, function (ko, Sortable) { "use strict"; var init = function (element, valueAccessor, allBindings, viewModel, bindingContext, sortableOptions) { From ac85bf3c1a112a317a9d4c7bd0c056744de67f8f Mon Sep 17 00:00:00 2001 From: dev101 Date: Fri, 22 Jan 2016 19:35:18 +0300 Subject: [PATCH 3/3] Revert "Module definition improved in knockout-sortable.js" --- knockout-sortable.js | 65 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 13 deletions(-) diff --git a/knockout-sortable.js b/knockout-sortable.js index 181a42b9b..dcb470a83 100644 --- a/knockout-sortable.js +++ b/knockout-sortable.js @@ -1,17 +1,56 @@ -(function (root, factory) { - if (typeof define === 'function' && define.amd) { - // AMD. Register as an anonymous module. - define(['knockout', 'Sortable'], factory); - } else if (typeof module === 'object' && module.exports) { - // Node. Does not work with strict CommonJS, but - // only CommonJS-like environments that support module.exports, - // like Node. - module.exports = factory(require('knockout'), require('sortablejs')); - } else { - // Browser globals (root is window) - root.returnExports = factory(root.ko, root.Sortable); +(function (factory) { + "use strict"; + //get ko ref via global or require + var koRef; + if (typeof ko !== 'undefined') { + //global ref already defined + koRef = ko; + } + else if (typeof require === 'function' && typeof exports === 'object' && typeof module === 'object') { + //commonjs / node.js + koRef = require('knockout'); + } + //get sortable ref via global or require + var sortableRef; + if (typeof Sortable !== 'undefined') { + //global ref already defined + sortableRef = Sortable; + } + else if (typeof require === 'function' && typeof exports === 'object' && typeof module === 'object') { + //commonjs / node.js + sortableRef = require('sortablejs'); + } + //use references if we found them + if (koRef !== undefined && sortableRef !== undefined) { + factory(koRef, sortableRef); + } + //if both references aren't found yet, get via AMD if available + else if (typeof define === 'function' && define.amd){ + //we may have a reference to only 1, or none + if (koRef !== undefined && sortableRef === undefined) { + define(['./Sortable'], function(amdSortableRef){ factory(koRef, amdSortableRef); }); + } + else if (koRef === undefined && sortableRef !== undefined) { + define(['knockout'], function(amdKnockout){ factory(amdKnockout, sortableRef); }); + } + else if (koRef === undefined && sortableRef === undefined) { + define(['knockout', './Sortable'], factory); + } + } + //no more routes to get references + else { + //report specific error + if (koRef !== undefined && sortableRef === undefined) { + throw new Error('knockout-sortable could not get reference to Sortable'); + } + else if (koRef === undefined && sortableRef !== undefined) { + throw new Error('knockout-sortable could not get reference to Knockout'); + } + else if (koRef === undefined && sortableRef === undefined) { + throw new Error('knockout-sortable could not get reference to Knockout or Sortable'); + } } -})(this, function (ko, Sortable) { +})(function (ko, Sortable) { "use strict"; var init = function (element, valueAccessor, allBindings, viewModel, bindingContext, sortableOptions) {