Community
Participate
Working Groups
Using Orion HEAD 01-27-2012-1939 1. Go to any of these pages: welcome (index.html), Settings, Sites, or Repositories. 2. Press 't'. 3. The "Find Files Named" dialog doesn't show up, and you get an error. Uncaught TypeError: Cannot call method 'getFavorites' of undefined OpenResourceDialog.js:146 The favorites service is missing on these pages.
Looks like this is due to a recent change I submitted in Bug 347058.
Created a new branch here: https://github.com/kdvolder/orion.client/tree/Bug370030 that has a commit: https://github.com/kdvolder/orion.client/commit/ae396eae8df9e2a7b51cabda001736c89b407cf1 that fixes the problem. I struggled a little bit on the best way to fix the problem. It seems that the favorites are only instantiated in setup.js and table.js. Both of those are files with lots of dependencies and so I didn't want to add a dependency to either of them in OpenResourcesDialog.js if I didn't have to. So, now the fix has in OpenResourcesDialog.js, if the favorites service is not already instantiated, it gets instantiated. The only potential issue with this is that it feels strange to instantiate a service in UI code like this. The fix works, but John if you think this is not appropriate, I can work on something else.
I think the general approach so far is that services are all instantiated by the "glue code" script at the root of each page (scripts of the form web/<page-name>/<page-name>.js). I think we realize this isn't an ideal solution but at this point it's probably better to stay consistent and avoid scattering service instantiation around different parts of the code. We have been trying to move away from passing the service registry around and instead just "inject" the required services into each script/module to keep things simple and make scripts like OpenResourceDialog.js more reusable. Susan/Simon any thoughts on this?
(In reply to comment #3) >Susan/Simon > any thoughts on this? the old bug on that issue is bug 337740. We should probably have the discussion there. We have some components (such as editor) that don't even know about the service registry because they are designed to be used without the orion core. But there are issues, let's talk about them in that bug
I just ran into a similar situation working on bug 349785, where we want to be able to add a favorite from the editor, or really from any page that is showing a resource. Pages without the favorites service will simply fail when you press the star button. So now we have two cases of a global command that needs a service, but we really don't need the service until the user chooses the command. I wonder in these two cases if the command implementation itself should check for the service and register it if it is not there. It seems really cumbersome and wrong from a dependency standpoint that we would go to every single page and update the glue to add this service.
I knew I was having a flashback. See bug 339509 and bug 341350. The problem back then was one of module requirements. We have fixed this with require.js but now we have the same problem with service initialization requirements. So it seems to me that we are missing a representation of what services need to be initialized for a particular module to work. But I would think we would want to keep it lazy. Simon?
(In reply to comment #5) > I wonder in these two cases if the command implementation itself should check > for the service and register it if it is not there. It seems really cumbersome > and wrong from a dependency standpoint that we would go to every single page > and update the glue to add this service. talking to myself, sorry. In the open resource case, the command implementation shouldn't have to know that open resource decided to implement favorites support. So I'm thinking that Andrew's approach is somewhat appropriate, lacking some generic way for a UI element to say "I need this list of services registered in order to do my work." In the "mark favorite" command case, the command does indeed know that it needs the favorites service, so you could expect it to register the service. What an ugly pattern, though... If it's not too expensive to register the service, maybe we should indeed invent some convention where modules declare what services they assume to be registered and we register them.
When I first delved into the details, I was surprised that services do not register themselves. I had originally assumed that just by ensuring that a js script that defines a service, then that service is available from the registry. Is there any reason why services don't register themselves? Doing so would ensure that all services are singletons, and make it easier to manage which services are actually available at any given page. It is likely that I am missing something about the architecture that makes this a bad idea, but from what I've seen so far, this does seem like a simpler solution.
I fixed this here: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=1473ea213a544d8cad4166ef03e7cd1ab407d533 It still didn't feel right to be making the decision on what service instance to use that deep in the code, but that is covered by bug 337740.
(In reply to comment #8) > Is there any reason why services don't register themselves? Doing so would > ensure that all services are singletons, and make it easier to manage which > services are actually available at any given page. Because singleton services are not very useful. The point of a service is that implementations are interchangeable and the consumer doesn't care which implementation they get. If each service registers itself, then there is no way for an application to make the configuration decision about which particular implementation of a given service it wants to use. Having said that I don't think we've figured out the right way to handle this in JavaScript yet.
> Because singleton services are not very useful. The point of a service is that > implementations are interchangeable and the consumer doesn't care which > implementation they get. If each service registers itself, then there is no way > for an application to make the configuration decision about which particular > implementation of a given service it wants to use. Having said that I don't > think we've figured out the right way to handle this in JavaScript yet. Makes sense why there is a need for this flexibility. I am still not sure how to address this problem for the OpenResourcesDialog. It seems to me that services should not be created until/unless they are needed. So, the first time a service is asked for, if no appropirate instances of that service exists, then (and only then) should the application try to create the service. But, I don't know what this would look like in the code.
