Merge branch 'fix/plugins_order'

lib/plugins/__tests__/listAll.js

@@ -1,36 +1,38 @@
 var PluginDependency = require('../../models/pluginDependency');
 var listAll = require('../listAll');
+var toNames = require('../toNames');
 
 describe('listAll', function() {
     it('must list default', function() {
         var deps = PluginDependency.listFromString('ga,great');
         var plugins = listAll(deps);
+        var names = toNames(plugins);
 
-        expect(plugins.size).toBe(8);
-
-        expect(plugins.has('ga')).toBe(true);
-        expect(plugins.has('great')).toBe(true);
-
-        expect(plugins.has('search')).toBe(true);
+        expect(names).toEqual([
+            'ga', 'great',
+            'highlight', 'search', 'lunr', 'sharing', 'fontsettings',
+            'theme-default' ]);
     });
 
     it('must list from array with -', function() {
         var deps = PluginDependency.listFromString('ga,-great');
         var plugins = listAll(deps);
+        var names = toNames(plugins);
 
-        expect(plugins.size).toBe(7);
-
-        expect(plugins.has('ga')).toBe(true);
-        expect(plugins.has('great')).toBe(false);
+        expect(names).toEqual([
+            'ga',
+            'highlight', 'search', 'lunr', 'sharing', 'fontsettings',
+            'theme-default' ]);
     });
 
     it('must remove default plugins using -', function() {
         var deps = PluginDependency.listFromString('ga,-search');
         var plugins = listAll(deps);
+        var names = toNames(plugins);
 
-        expect(plugins.size).toBe(6);
-
-        expect(plugins.has('ga')).toBe(true);
-        expect(plugins.has('search')).toBe(false);
+        expect(names).toEqual([
+            'ga',
+            'highlight', 'lunr', 'sharing', 'fontsettings',
+            'theme-default' ]);
     });
 });

lib/plugins/__tests__/sortPlugins.js

@@ -1,50 +1,45 @@
-var PluginDependency = require('../../models/pluginDependency');
+var Immutable = require('immutable');
+
+var Plugin = require('../../models/plugin');
 var sortPlugins = require('../sortPlugins');
-var listAll = require('../listAll');
+var toNames = require('../toNames');
 
 describe('sortPlugins', function() {
     it('must load themes after plugins', function() {
-        var deps = PluginDependency.listFromString('theme-faq'),
-            allPlugins = listAll(deps);
+        var allPlugins = Immutable.OrderedMap([
+            ['hello', Plugin.createFromString('hello')],
+            ['theme-test', Plugin.createFromString('theme-test')],
+            ['world', Plugin.createFromString('world')]
+        ]);
 
-        var sorted = sortPlugins(allPlugins, []);
-        var names = sorted
-            .map(function(plugin) {
-                return plugin.getName();
-            })
-            .toArray();
+        var sorted = sortPlugins(allPlugins);
+        var names = toNames(sorted);
 
         expect(names).toEqual([
-            'fontsettings',
-            'sharing',
-            'lunr',
-            'search',
-            'highlight',
-            'theme-faq',
-            'theme-default'
+            'hello',
+            'world',
+            'theme-test'
         ]);
     });
 
-    it('must load themes after plugins with a complex dependencies list', function() {
-        var deps = PluginDependency.listFromString('comment,theme-faq,-search,ga'),
-            allPlugins = listAll(deps);
+    it('must keep order of themes', function() {
+        var allPlugins = Immutable.OrderedMap([
+            ['theme-test', Plugin.createFromString('theme-test')],
+            ['theme-test1', Plugin.createFromString('theme-test1')],
+            ['hello', Plugin.createFromString('hello')],
+            ['theme-test2', Plugin.createFromString('theme-test2')],
+            ['world', Plugin.createFromString('world')]
+        ]);
 
-        var sorted = sortPlugins(allPlugins, []);
-        var names = sorted
-            .map(function(plugin) {
-                return plugin.getName();
-            })
-            .toArray();
+        var sorted = sortPlugins(allPlugins);
+        var names = toNames(sorted);
 
         expect(names).toEqual([
-            'ga',
-            'comment',
-            'fontsettings',
-            'sharing',
-            'lunr',
-            'highlight',
-            'theme-faq',
-            'theme-default'
+            'hello',
+            'world',
+            'theme-test',
+            'theme-test1',
+            'theme-test2'
         ]);
     });
 });

lib/plugins/listAll.js

@@ -2,6 +2,7 @@ var Immutable = require('immutable');
 var Plugin = require('../models/plugin');
 
 var DEFAULT_PLUGINS = require('../constants/defaultPlugins');
+var sortPlugins = require('./sortPlugins');
 
 /**
     List all plugins for a book
@@ -20,7 +21,7 @@ function listAll(deps) {
         });
 
     // Concat with default plugins
-    deps = DEFAULT_PLUGINS.concat(deps);
+    deps = deps.concat(DEFAULT_PLUGINS);
 
     // Convert to an ordered map of Plugin
     var plugins = deps
@@ -36,7 +37,8 @@ function listAll(deps) {
         return toRemove.includes(name);
     });
 
-    return plugins;
+    // Sort plugins
+    return sortPlugins(plugins);
 }
 
 module.exports = listAll;

lib/plugins/loadForBook.js

@@ -3,7 +3,6 @@ var Promise = require('../utils/promise');
 var listForBook = require('./listForBook');
 var findForBook = require('./findForBook');
 var loadPlugin = require('./loadPlugin');
-var sortPlugins = require('./sortPlugins');
 
 
 /**
@@ -27,9 +26,6 @@ function loadForBook(book) {
             );
         });
 
-        // Sort plugins
-        installed = sortPlugins(installed, requirementsKeys);
-
         // Log state
         logger.info.ln(installed.size + ' plugins are installed');
         if (requirements.size != installed.size) {

lib/plugins/sortPlugins.js

@@ -1,12 +1,19 @@
+var Immutable = require('immutable');
+
 var THEME_PREFIX = require('../constants/themePrefix');
-var LOADING_ORDER = ['plugin', 'theme'];
+
+var TYPE_PLUGIN = 'plugin';
+var TYPE_THEME  = 'theme';
+
 
 /**
  * Returns the type of a plugin given its name
+ * @param {Plugin} plugin
  * @return {String}
  */
-function pluginType(name) {
-    return (name && name.indexOf(THEME_PREFIX) === 0) ? 'theme' : 'plugin';
+function pluginType(plugin) {
+    var name = plugin.getName();
+    return (name && name.indexOf(THEME_PREFIX) === 0) ? TYPE_THEME : TYPE_PLUGIN;
 }
 
 
@@ -15,22 +22,15 @@ function pluginType(name) {
  * The themes should always be loaded after the plugins
  *
  * @param {<OrderedMap<String:Plugin>>} plugins
- * @param {<List<String>>} requirementsKeys
  * @return {OrderedMap<String:Plugin>}
  */
-function sortPlugins(plugins, requirementsKeys) {
-    // Sort plugins to match list in book.json
-    return plugins.sort(function(a, b) {
-        // Get order from book.json
-        var definitionOrder = requirementsKeys.indexOf(a.getName()) < requirementsKeys.indexOf(b.getName());
+function sortPlugins(plugins) {
+    var byTypes = plugins.groupBy(pluginType);
 
-        // Get order from plugins a and b type
-        var aType = pluginType(a.getName()),
-            bType = pluginType(b.getName()),
-            loadingOrder = LOADING_ORDER.indexOf(aType) < LOADING_ORDER.indexOf(bType);
-
-        return loadingOrder || definitionOrder ? -1 : 1;
-    });
+    return byTypes.get(TYPE_PLUGIN, Immutable.OrderedMap())
+        .merge(
+            byTypes.get(TYPE_THEME, Immutable.OrderedMap())
+        );
 }
 
 module.exports = sortPlugins;

lib/plugins/toNames.js

@@ -0,0 +1,16 @@
+
+/**
+ * Return list of plugin names. This method is nly used in unit tests.
+ *
+ * @param {OrderedMap<String:Plugin} plugins
+ * @return {Array<String>}
+ */
+function toNames(plugins) {
+    return plugins
+        .map(function(plugin) {
+            return plugin.getName();
+        })
+        .toArray();
+}
+
+module.exports = toNames;