Repository: opentable/oc
Pull request #795 (2017-12-21 20:20:42)

 [DX-324] full async plugins initialisation
The reason here is an old design decision. I think that given the way plugins evolved, it makes much more sense to wait express to start after all the plugins are fully initialised.


Patch:
diff --git a/src/registry/domain/plugins-initialiser.js b/src/registry/domain/plugins-initialiser.js
index aeeded02..4996f4c9 100644
--- a/src/registry/domain/plugins-initialiser.js
+++ b/src/registry/domain/plugins-initialiser.js
@@ -81,8 +81,10 @@ module.exports.init = function(pluginsToRegister, callback) {
   };
 
   const loadPlugin = function(plugin, cb) {
+    const done = _.once(cb);
+
     if (registered[plugin.name]) {
-      return cb();
+      return done();
     }
 
     if (!plugin.register.dependencies) {
@@ -90,18 +92,17 @@ module.exports.init = function(pluginsToRegister, callback) {
     }
 
     if (!dependenciesRegistered(plugin.register.dependencies)) {
-      return defer(plugin, cb);
+      return defer(plugin, done);
     }
 
     const dependencies = _.pick(registered, plugin.register.dependencies);
 
-    plugin.register.register(
-      plugin.options || {},
-      dependencies,
-      plugin.callback || _.noop
-    );
-    registered[plugin.name] = plugin.register.execute;
-    cb();
+    plugin.register.register(plugin.options || {}, dependencies, err => {
+      const pluginCallback = plugin.callback || _.noop;
+      pluginCallback(err);
+      registered[plugin.name] = plugin.register.execute;
+      done(err);
+    });
   };
 
   const terminator = function(err) {


Test patch:
diff --git a/test/unit/registry-domain-plugins-initialiser.js b/test/unit/registry-domain-plugins-initialiser.js
index 029c8743..a08e4e7d 100644
--- a/test/unit/registry-domain-plugins-initialiser.js
+++ b/test/unit/registry-domain-plugins-initialiser.js
@@ -105,29 +105,24 @@ describe('registry : domain : plugins-initialiser', () => {
         {
           name: 'getValue',
           register: {
-            register: function(options, deps, cb) {
+            register: (options, deps, cb) => {
               passedOptions = options;
               cb();
             },
-            execute: function(key) {
-              return passedOptions[key];
-            }
+            execute: key => passedOptions[key]
           },
-          options: {
-            a: 123,
-            b: 456
-          }
+          options: { a: 123, b: 456 }
         },
         {
           name: 'isFlagged',
           register: {
-            register: function(options, deps, cb) {
-              flag = true;
-              cb();
+            register: (options, deps, cb) => {
+              setTimeout(() => {
+                flag = true;
+                cb();
+              }, 10);
             },
-            execute: function() {
-              return flag;
-            }
+            execute: () => flag
           }
         }
       ];
@@ -148,8 +143,8 @@ describe('registry : domain : plugins-initialiser', () => {
     });
 
     it('should be make the functionality usable', () => {
-      const a = result.getValue('a'),
-        flagged = result.isFlagged();
+      const a = result.getValue('a');
+      const flagged = result.isFlagged();
 
       expect(a).to.equal(123);
       expect(flagged).to.equal(true);