Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin as function #89

Closed
jorgecuesta opened this issue Jan 20, 2017 · 11 comments
Closed

Plugin as function #89

jorgecuesta opened this issue Jan 20, 2017 · 11 comments
Labels
feature New functionality or improvement
Milestone

Comments

@jorgecuesta
Copy link
Contributor

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 csrl added feature New functionality or improvement request labels Jan 26, 2017
@csrl
Copy link
Contributor

csrl commented Jan 26, 2017

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
Copy link
Contributor

csrl commented Jan 26, 2017

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
Copy link
Contributor Author

@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
Copy link
Contributor

csrl commented Mar 20, 2017

@jorgecuesta are you still interested in this?

@jorgecuesta
Copy link
Contributor Author

@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
Copy link
Contributor

csrl commented Mar 21, 2017

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

@jorgecuesta
Copy link
Contributor Author

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

@csrl
Copy link
Contributor

csrl commented Mar 21, 2017

sure, no problem.

@jorgecuesta
Copy link
Contributor Author

@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
Copy link

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.

@csrl csrl added this to the 4.2.0 milestone Jul 28, 2017
@Marsup Marsup removed the request label Sep 21, 2019
@lock
Copy link

lock bot commented Jan 9, 2020

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

4 participants