Allow service factory functions to return promises #3195
Conversation
|
@ctrahey thnx for this pull request. I can see what you are trying to do but the current pull request is not in the state that would allow us to consider for merging as it is missing unit tests and documentation updates. What worries is the most, though, is that would create API where a method can either return a real value (synchronous) or a promise (async) depending on its arguments... See some more comments in the code. |
|
Unit tests: Great feedback, will generate. Thanks! With respect to the non-deterministic synchronicity of the API: I really appreciate this concern. I'll admit to being relatively new (8 weeks or so) to Angular, and I wonder if we could discuss the optimal way to achieve what I'm going for here. Perhaps that would be elsewhere in the code, or perhaps it would be going full commitment to promise-based invoke() (I would have the Angular team guide that thinking) Primary goals (as you likely inferred):
An idea: Example: |
|
@ctrahey I would like to sound more welcoming but I'm afraid that more things would have to change in order to support services that can be injected with arguments that are resolved in the async way. Let me ask this - how would you create a promise in your myModule.provide('myService', function(){
return {
$asyncGet: function(){
// return a promise here - OK - but how to get an instance of a promise???
}
}
} |
|
Hm, you'll need to forgive me, I was using However, I realize that the iteration on the idea doesn't really solve the problem. If I might attempt to articulate what I perceive is the issue you are addressing (looking for your confirmation):
Is there a larger point to be discussed about the purpose (and use cases) for async-fulfilled dependencies? At a high level, I see the contract with the user like this (and I recognize that
I recognize a few assumptions in my thinking, I'd love to refine my understanding so that I can build a contribution that effectively facilitates this potentially powerful feature:
This is my thinking, anyway. With respect to the Meta-conversation: Please advise if this conversation is more appropriate to take place elsewhere, and if we think there is a chance in hell of a change like this becoming a reality. |
|
I think this is kind of what @Iminar was doing with his new async On 11 July 2013 23:36, ctrahey notifications@github.com wrote:
|
PR Checklist (Major Feature)
|
|
@ctrahey - thanks for this PR. I think it is a really interesting idea. We need to get the core team to have a good look at this as I think there are a number of ramifications. |
|
@petebacondarwin Thank you for looking at this. I'm excited about the work I've built on top of this concept and I'd love to be a contributor to Angular. I've completed the CLA, and should be able to address some of the minor tweaks discussed here sometime this weekend. I appreciate any feedback on the code or the process of contributing. Thank you! |
In Angular.toJson, any properties with a leading '$' character will be stripped from the resulting string since angular uses this notation internally for services. There have been complaints of not knowing about this functionality until it breaks within their code.
This plugin is shipped as a default one with Karma. It's specified as a peer dependency. I assume, there's an old version of NPM on the CI server, which does not support peerDependencies and therefore it didn't get installed. This will make the dependency explicit.
parseKeyValue and toKeyValue can now handle duplicate values in the query.
```
?x=1&x=2 <-> {x:[1,2]}
```
The algorithm looks like:
1)parseKeyValue looks for presence of obj[key]
2)detects and replaces obj[key] with [obj[key],val]
3)then pushes more duplicates if necessary
4)toKeyValue decodes array correctly
5)(not changed)$location.search({param: 'key'}) still replaces if necessary
6)(not changed)$location.search({param: ['key1', 'key2']}) sets the url with duplicates
BREAKING CHANGE: Before this change:
- `parseKeyValue` only took the last key overwriting all the previous keys;
- `toKeyValue` joined the keys together in a comma delimited string.
This was deemed buggy behavior. If your server relied on this behavior
then either the server should be fixed or a simple serialization of
the array should be done on the client before passing it to $location.
This reverts commit 0c6fb66. The change invalidated the test because the point of the the test was to test that an element directive works. Changing it to attribute directive was wrong.
- parallelize the tasks - cache requests (e2e tests) This reduces the time from ~18min to ~12min. It makes the output little messy. We could buffer output of each task and display it once it's fully finished, nicely. I think giving instant feedback is better.
The quotes were causing the minerr processing to fail on Windows
This reverts commit 15e1a29. The original commit was fixing two issues - one of them was preventing attributes that triggered directives that replaced the compiled node to be merged into the new node. This change was a breaking change (as seen in the diff of the tests in this commit) and that's why it's being removed. A proper fix will follow.
Previously if a template contained a directive that had a template (sync or async) and the directive template was to replace the original element and the directive template contained another directive on the root element of this template and this new directive was an element transclude directive then an infinite recursion would follow because the compiler kept on re-adding and reapplying the original directive to the replaced node. This change fixes that. Closes angular#2155
Add tick and cross mark corresponding to their respective unicodes.
Previously an element like
<div class="foo ng-cloak">...</div>
would still be annoyingly visible if it matched a CSS rule like
.foo { display: inline-block; }, overriding ng-cloak's display: none.
Match jQuery behavior when prepending array into empty element
Enhances sandboxing of Angular Expressions to prevent attacks via:
{}.toString.constructor(alert("evil JS code"))
Regular expression objects didn't used to be considered to be equal when using 'angular.equals'. Dirty checking therefore failed to recognize a property modification. Closes angular#2685
caching the version number speeds up the build and preserves resources. this also fixed EMFILE error that now occurs on some macs.
Add an alternative way to define a mapping for ng:pluralize using attributes instead of the `when` attribute Closes angular#2454
It is now possible to notify a promise through deferred.notify() method. Notifications are useful to provide a way to send progress information to promise holders.
Google chrome (when tested using karma) spits out a log file called libpeerconnection.log
…that is not attached to the DOM
Fix closing parenthesis, quotes around string literal and remove trailing whitespace. Closes angular#3250
Added directives for focus and blur events. Closes angular#1277
Ref: 1adf29a BREAKING CHANGE: img[src] URLs are now sanitized via a separate whitelist regex instead of sharing the whitelist regex with a[href]. With this change, img[src] URLs may also be data: URI's matching mime types image/*. mailto: URLs are disallowed (and do not make sense for img[src] but were allowed under the a[href] whitelist used before.)
This allows us to use minErr in other modules, such as resource and sanitize.
The input [number] error spans did not show on the example, as they were relying on an non-existing property (myForm.list.$error) vs the working property (myForm.input.$error)
|
Wow, sorry about the mess. @petebacondarwin @pkozlowski-opensource and others, Please note that I've opened a cleaned up PR here: #3295 Which includes docs changes, Unit Tests, and the suggested $q.all() change. |
This change to $injector.invoke allows user-defined services to return promises from their factory functions.
Any guidance is most welcome. Here's the motivation:
I have a service that needs to do an asynchronous task before it even knows what methods it will offer (think of it like a proxy for a WSDL--defined service or something like that). I would like this to be highly re-useable (i.e. I mean "proxy" in the sense that I am not the author of the "logical" service that the users will be consuming), so I do no want users to work with their injected dependencies in a new way (e.g. promises).
Enter this change-set.
The main idea is that when the injector is asked to invoke something, it should "wait" (via it's own promise) until all of the invocation's dependencies have been resolved before performing it so that user-land code always gets resolved services, not promises. This, of course, will allow us to write service factory functions that return promises, and thus have "asynchronously constructed" services. This opens the door to the use of some advanced service-level architectures.
I'm happy to work on this further (e.g. provide tests), but I would love some feedback on the bigger picture here.
Thanks!