feat($resource): deep-merge custom actions with default actions. #14821
Conversation
gabrielmaldi
force-pushed
the
resource-merge-actions
branch
from
3e2bfd4 to
2c7440f
Compare
Previously, if you wanted to specify a timeout, transform, interceptor, or any
other property for an action, you also had to include the other properties of
the default action (method, isArray):
```
var Resource = $resource('/foo', {}, { save: { method: 'POST', timeout: 10000 }});
```
Now, a deep-merge between the provided custom actions and the default actions
will be performed, so you can specify just the properties you want to add or
override:
```
var Resource = $resource('/foo', {}, { save: { timeout: 10000 }});
```
Closes angular#14821
|
TBH, I am not a fan of this change. It would save a few characters for a few usecases and add a few characters for a few other usecases. But it would increase the complexity/reduce the declarativeness in all usecases: In order to know what some method of a $resource class/instance does, one would have to look up the default action methods (which might or might not be overriden). I don't think that the few usecases that would benefit from this PR, justify the extra cost. I will leave it open to see what others think. |
|
I get your point. What I think is that most devs would expect actions to work this way, which is what happened to me: I didn't understand why setting a timeout on |
|
I'm kind of on the fence about this. It does make sense to expect that setting the options preserve the defaults, especially in cases like "save" where it is unlikely that the verb is changed. Then again, it doesn't seem to be a huge problem, as I've never seen this mentioned before. |
gabrielmaldi
force-pushed
the
resource-merge-actions
branch
from
2c7440f to
8246c36
Compare
Previously, if you wanted to specify a timeout, transform, interceptor, or any
other property for an action, you also had to include the other properties of
the default action (method, isArray):
```
var Resource = $resource('/foo', {}, { save: { method: 'POST', timeout: 10000 }});
```
Now, a deep-merge between the provided custom actions and the default actions
will be performed, so you can specify just the properties you want to add or
override:
```
var Resource = $resource('/foo', {}, { save: { timeout: 10000 }});
```
Closes angular#14821
Previously, if you wanted to specify a timeout, transform, interceptor, or any
other property for an action, you also had to include the other properties of
the default action (method, isArray):
```
var Resource = $resource('/foo', {}, { save: { method: 'POST', timeout: 10000 }});
```
Now, a deep-merge between the provided custom actions and the default actions
will be performed, so you can specify just the properties you want to add or
override:
```
var Resource = $resource('/foo', {}, { save: { timeout: 10000 }});
```
Closes angular#14821
gabrielmaldi
force-pushed
the
resource-merge-actions
branch
from
8246c36 to
32c35f1
Compare
|
Also, if, for instance, you add a custom header to |
|
I would not introduce a breaking change so that I "may someday save someone some time" 😛 |
|
I agree with @gkalpak that this might cause more problems than it solves. I've updated the documentation to clarify what happens when the custom action has the same key as the default action. |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature.
What is the current behavior? (You can also link to an open issue here)
If you want to specify a timeout, transform, interceptor, or any other property for an action, you also have to include the other properties of the default action (method, isArray):
What is the new behavior (if this is a feature change)?
Perform a deep-merge between the provided custom actions and the default actions, so you can specify just the properties you want to add or override:
Does this PR introduce a breaking change?
Theoretically yes, but should have no impact in practice.
Please check if the PR fulfills these requirements