hapijs / glue

Server composer for hapi.js
Other
245 stars 62 forks source link

Plugin as function #89

Closed jorgecuesta closed 7 years ago

jorgecuesta commented 7 years ago

Hi I'm want ask if you can add a way on registrations.plugin.register to receive function like service.register.

The point of it is because I need have require/imports statements on top of manifest to can be loaded / parsed etc for server rendering purposes on some routes attached as plugins.

csrl commented 7 years ago

Seems reasonable to me. If the plugin register field is a function, it should use it as is, otherwise if its a string pass it to require().

csrl commented 7 years ago

This should be what you want. Give it a try and let me know. If someone has time to write the test case, feel free to generate a commit from this and submit a pull request. Else I'll get to it sometime. Although now that I think about it, the schema validation also needs updated.

diff --git a/lib/index.js b/lib/index.js
index 402ad47..2431857 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -177,11 +177,14 @@ internals.parsePlugin = function (plugin, relativeTo) {
         plugin = { register: plugin };
     }

-    let path = plugin.register;
-    if (relativeTo && path[0] === '.') {
-        path = Path.join(relativeTo, path);
+    if (typeof plugin.register === 'string') {
+        let path = plugin.register;
+        if (relativeTo && path[0] === '.') {
+            path = Path.join(relativeTo, path);
+        }
+
+        plugin.register = require(path);
     }

-    plugin.register = require(path);
     return plugin;
 };
jorgecuesta commented 7 years ago

@csrl I will try to make this tonight and let you know. About schema update I think like you. Hapi has a lot of great updates and fixes. Best regards.

csrl commented 7 years ago

@jorgecuesta are you still interested in this?

jorgecuesta commented 7 years ago

@csrl Sorry about delay, I can not test your change, because make a simple change on app logic to prevent this issue but should be great don't need any kind of trick to solve this situation. Need some help or test from me? I can do it tonight...

csrl commented 7 years ago

It needs doc update, schema validation update, and test cases written for it.

jorgecuesta commented 7 years ago

Ok I will try to do everything you need but you should check my english because it isn't my best skill.

csrl commented 7 years ago

sure, no problem.

jorgecuesta commented 7 years ago

@csrl Finally I put a pull request with all the changes required by you. Let me know if can I do any more for this awesome library ;)

sberryman commented 7 years ago

This is NOT closed as the PR hasn't been merged yet. This would be very nice to merge as I've recently come across a project which doesn't export a plugin function.

https://github.com/openzipkin/zipkin-js/blob/master/packages/zipkin-instrumentation-hapi

They use module.exports.hapiMiddleware which is extremely annoying but valid. As a workaround I have to use a string with the full path to the location of the plugin function.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.