-
Notifications
You must be signed in to change notification settings - Fork 82
adding the model suffix #251
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
Conversation
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
Hello @Traksewt, thank you for the pull request! I believe your needs should be already met by the recently added options "includeCommonModules", "namespaceModels" and "namespaceDelimiter" - see #219 for more details. Could you please give it a try and let us know how far you could get? If the current solution is not good enough, then I am proposing to stay consistent with the approach used in #219 and use Also the proposed implementation is repeating name building too much. Could you please rework it to build the core model names (Auth, Resource, ResourceProvider, AuthRequestInterceptor) only once? - LoopBack<%-: loopbackModelSuffix%>Auth
+<%-: loopbackAuth %> @keithajackson what's your opinion on the change proposed in this pull request? |
@slnode ok to test |
Thanks for the feedback. A problem with excluding the common modules I can look into reusing the ngModuleName, however, while dots in module I can look at reworking the core model names in JS only once. |
I see, this makes a lot of sense!
This looks reasonable to me too. What I would like to do is to come up with a nice consistent solution that would address all different needs related to using multiple LoopBack clients in the same application. So far, I see the following "requirements":
In that light, I am proposing to slightly change your proposal to the following:
As I am re-reading my comment, the second option seems the best to me. I am imagining that when generating "Accounts" client, one will set @keithajackson I would really like to get some input from you, as you are using multiple Angular clients too. What do you think? |
@bajtos OK option 2 looks fairly good and neat. The one issue I have is that the style guides encourage to use dots for submodules in module names, but dots are not pretty for service names. I could strip out the dot to work out the prefix, though that is a little obscure for the prefix generation (the user will need to know this so they can set the BaseURL). I'm talking about dots within the module name itself, I'm not talking about any delimiter between the new prefix (as you mentioned, there won't be any delimiter). So I see: |
Hey all! Great discussion here - here's my late two cents: If you're separately auth-ing in to two different LoopBack APIs, then this namespacing request totally makes sense. Minus hostname confusion (see below), the solution of allowing (optional) common module namespacing sounds good to me. However: I see a hostname confusion edge case that could cause hard-to-trace failures if you are using multiple APIs hosted behind a proxy (which I'm guessing most of our sites in production will). If you have a FE connecting to:
something weird happens at lines 373-377 from the generator:
Both URLs will resolve to the same host per the Possible solutions I can think of: make urlBaseHost a predetermined value (which defaults to the current
As far as I can tell, this should result in a non-breaking change that allows developers that want to auth against multiple API's to provide their own (more specific) base url strings, such as As to the namespacing: please clarify if I missed something, but John Papa's comment on using dots for submodules doesn't apply here, since everything goes into the same module (
I should get a styleguide-compliant naming system:
I really like @bajtos ' idea of just having one prefix, and the idea of auto-converting between dot and camel case worries me (auto-reformtting variable names/delimiters has been one of the most confusing "necessary evil" parts of Angular 1 itself in my experience). |
Thank you @keithajackson for your thoughtful comment. What's the best way to move this forward then? @Traksewt @keithajackson |
Can one of the admins verify this patch? |
OK, looking out for urlBaseHost sounds right.
If urlBaseHost is going to contain path elements, could we just use urlBase instead? Or could urlBase be relative? If so, then will this give what we need?
Regarding the dots, that is correct that everything goes into the one module, but the problem is if that module name actually does have a dot in it. E.g. if our generated module name is called 'Farm.Accounts', Then when you inject that and prepend it to a service, you will get a dot in the service name. The easy answer is a), say you don't support it, but I (and possibly others in the future) will need to change our names which deviates from our standards. Not the end of the world, but just the generator won't be following best practices. We are using multiple loopbacks for many micro-services, so having some structure is helpful to us. |
I see. Can we simply remove the dots from the generated service name? Or is that too hacky and/or difficult to grasp for SDK users?
I can also imagine that we can implement this part by replacing dots with the configured
Thoughts? To be honest, I don't have very strong opinion here and I am happy to accept any reasonable solution that supports all different use cases outlined in the previous discussion. |
Hello, what's the status of this pull request? |
I am closing this pull request as abandoned. Feel free to reopen if/when you get time to work on it again. |
PR Finished in #269 |
Add a suffix to the core loopback models to allow multiple lb services to exist on the same client, pointing to APIs on different services.
The core models start with 'LoopBack'. The suffix is appended to the LoopBack part of the name while still appending its class after the suffix.
For example if the suffix is Accounts, then the generated core loopback models are: LoopBackAccountsResource, LoopBackAccountsAuth, LoopBackAccountsResourceProvider, LoopBackAccountsAuthRequestInterceptor.
See Issue #250