-
-
Notifications
You must be signed in to change notification settings - Fork 751
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
Support for registering services dynamically #107
Conversation
For some reason I CAN create a branch on the main Feathers repo, but I'm not able to push to it, so there's a straggling branch called dynamic-services-2 hanging out right now. Update: Never mind, I wasn't able to push it, so there's no straggling branch after all. |
I just double checked and you should have push access to this repository. Did you use |
Ahh. I enabled two-factor auth. I forgot to use the personal access token. I pushed the branch. |
@daffl What did you have in mind? |
@@ -24,6 +24,19 @@ exports.setupMethodHandler = function setupMethodHandler (emitter, params, servi | |||
args.splice(position, 0, {}); | |||
} | |||
args[position] = _.extend({ query: args[position] }, params); | |||
|
|||
// If there was no callback supplied by a socket client... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this into a separate PR since it belongs to some different functionality?
Well it looks that for example the code from https://github.com/feathersjs/feathers/pull/107/files#diff-a89e33317f0c782705345838f29cf003R57 and https://github.com/feathersjs/feathers/pull/107/files#diff-dbb6eace96490fa4b721a67e6fd9340dR57 can be maybe consolidated into https://github.com/feathersjs/feathers/blob/master/lib/providers/socket/commons.js. I'm also wondering if we could somehow hook into app.service and make sure that the provider methods can be called at any time? I think you a currently doing this in |
3329797
to
288ca63
Compare
OK. I moved most of it into the commons. You'll notice that in the I left the |
Oh. I also took that other commit regarding socket callbacks out. |
I'll make a PR for it after we get this one merged. |
Sweet! I will check it out tonight and then merge it in. This will make for a great 1.1 release. Debating if ZeroRPC support would be something worth considering for that, too (and writing up how use this as microservices). Oh and I'd like to polish the website a little, too. |
Merged! Great job! |
…s-1.2.7 Update generator-feathers to the latest version 🚀
This is Dynamic Services on top of the recent 1.0.2 patch. I've also made it no longer a requirement to pass a callback (#96). If the request fails, it logs a message to the console for a developer to track down.
As for consolidating the code a bit more. This is basically creating a second path in the code to setup a service. The first way is through the setup methods, and now this for sure works after the server has started. I'm not sure if it would be possible to just use this new way BEFORE the server starts. I would have to check to see if there is anything that the setup methods put in place that the services require as prerequisites.
Update: We for sure have to wait until the providers are set in place, which isn't really guaranteed until after the server starts (since providers are setup with configure). There may be other optimizations that can be made?
Closes #67 and closes #82