From 1bf8b80cec0545ce893ea09290572809fe1e6d77 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Thu, 9 Jun 2016 22:05:55 +0000 Subject: [PATCH] Refactored converse-pluggable to remove all deferreds by not attempting to load `optional_dependencies` via require.js. Instead, we just expect them to be plugins and to have been loaded already. --- src/converse-core.js | 15 +++--- src/converse-muc.js | 9 ++-- src/converse-pluggable.js | 97 +++++++++++---------------------------- 3 files changed, 39 insertions(+), 82 deletions(-) diff --git a/src/converse-core.js b/src/converse-core.js index f8b6807e9..4d200cdcc 100755 --- a/src/converse-core.js +++ b/src/converse-core.js @@ -633,6 +633,11 @@ // XXX: Deprecate in favor of init_deferred this.callback(); } + if (converse.connection.service === 'jasmine tests') { + init_deferred.resolve(converse); + } else { + init_deferred.resolve(); + } converse.emit('initialized'); }; @@ -1780,15 +1785,9 @@ converse.pluggable.initializePlugins({ 'updateSettings': updateSettings, 'converse': converse - }).then(function () { - converse._initialize(); - converse.registerGlobalEventHandlers(); - if (converse.connection.service === 'jasmine tests') { - init_deferred.resolve(converse); - } else { - init_deferred.resolve(); - } }); + converse._initialize(); + converse.registerGlobalEventHandlers(); return init_deferred.promise(); }; return converse; diff --git a/src/converse-muc.js b/src/converse-muc.js index adc05e22f..f565fb6a0 100755 --- a/src/converse-muc.js +++ b/src/converse-muc.js @@ -43,14 +43,13 @@ Strophe.addNamespace('MUC_USER', Strophe.NS.MUC + "#user"); converse_api.plugins.add('converse-muc', { - /* Optional dependencies are require.js dependencies which might be - * overridden or relied upon if they exist, but otherwise ignored. + /* Optional dependencies are other plugins which might be + * overridden or relied upon, if they exist, otherwise they're ignored. * * However, if the setting "strict_plugin_dependencies" is set to true, - * then these dependencies will be considered required. + * an error will be raised if the plugin is not found. * - * Optional dependencies will be available in the initialize method as - * a the "optional_dependencies" attribute of the plugin. + * NB: These plugins need to have already been loaded via require.js. */ optional_dependencies: ["converse-controlbox"], diff --git a/src/converse-pluggable.js b/src/converse-pluggable.js index 41f273815..783ae8ad4 100644 --- a/src/converse-pluggable.js +++ b/src/converse-pluggable.js @@ -84,33 +84,23 @@ }.bind(this)); }, - setOptionalDependencies: function (plugin, dependencies) { - plugin.optional_dependencies = dependencies; - return plugin; - }, - - loadOptionalDependencies: function (plugins) { - var deferred = new $.Deferred(); - require(plugins, - function () { - _.each(plugins, function (name) { - var plugin = this.plugins[name]; - if (plugin) { - this.initializePlugin(plugin).then( - deferred.resolve.bind(this, plugins) - ); - } - }.bind(this)); - }.bind(this), - function () { - if (this.plugged.strict_plugin_dependencies) { - deferred.fail.apply(this, arguments); - this.throwUndefinedDependencyError(arguments[0]); - } else { - deferred.resolve.apply(this, [plugins]); + loadOptionalDependencies: function (plugin) { + _.each(plugin.optional_dependencies, function (name) { + var dep = this.plugins[name]; + if (dep) { + if (_.contains(dep.optional_dependencies, plugin.__name__)) { + // FIXME: circular dependency checking is only one level deep. + throw "Found a circular dependency between the plugins \""+ + plugin.__name__+"\" and \""+name+"\""; } - }.bind(this)); - return deferred.promise(); + this.initializePlugin(dep); + } else { + this.throwUndefinedDependencyError( + "Could not find optional dependency \""+name+"\" "+ + "for the plugin \""+plugin.__name__+"\". "+ + "If it's needed, make sure it's loaded by require.js"); + } + }.bind(this)); }, throwUndefinedDependencyError: function (msg) { @@ -140,7 +130,16 @@ }.bind(this)); }, - _initializePlugin: function (plugin) { + initializePlugin: function (plugin) { + if (_.contains(this.initialized_plugins, plugin.__name__)) { + // Don't initialize plugins twice, otherwise we get + // infinite recursion in overridden methods. + return; + } + _.extend(plugin, this.properties); + if (plugin.optional_dependencies) { + this.loadOptionalDependencies(plugin); + } this.applyOverrides(plugin); if (typeof plugin.initialize === "function") { plugin.initialize.bind(plugin)(this); @@ -148,55 +147,15 @@ this.initialized_plugins.push(plugin.__name__); }, - asyncInitializePlugin: function (plugin) { - var deferred = new $.Deferred(); - this.loadOptionalDependencies(plugin.optional_dependencies).then( - _.compose( - deferred.resolve, - this._initializePlugin.bind(this), - _.partial(this.setOptionalDependencies, plugin) - )); - return deferred.promise(); - }, - - initializePlugin: function (plugin) { - var deferred = new $.Deferred(); - if (_.contains(this.initialized_plugins, plugin.__name__)) { - // Don't initialize plugins twice, otherwise we get - // infinite recursion in overridden methods. - return deferred.resolve().promise(); - } - _.extend(plugin, this.properties); - if (plugin.optional_dependencies) { - this.asyncInitializePlugin(plugin).then(deferred.resolve); - } else { - this._initializePlugin(plugin); - deferred.resolve(); - } - return deferred.promise(); - }, - - initNextPlugin: function (remaining, deferred) { - if (remaining.length === 0) { - deferred.resolve(); - return; - } - var plugin = remaining.pop(); - this.initializePlugin(plugin).then( - this.initNextPlugin.bind(this, remaining, deferred)); - }, - initializePlugins: function (properties) { /* The properties variable is an object of attributes and methods * which will be attached to the plugins. */ - var deferred = new $.Deferred(); if (!_.size(this.plugins)) { - return deferred.promise(); + return; } this.properties = properties; - this.initNextPlugin(_.values(this.plugins).reverse(), deferred); - return deferred; + _.each(_.values(this.plugins), this.initializePlugin.bind(this)); } }); return {