Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

$provide.decorator to extend directive scope no longer works in 1.3.x #10149

Closed
sumigoma opened this issue Nov 20, 2014 · 17 comments
Closed

$provide.decorator to extend directive scope no longer works in 1.3.x #10149

sumigoma opened this issue Nov 20, 2014 · 17 comments

Comments

@sumigoma
Copy link

I'm using a directive that has an isolated scope, but I would like to expose an additional member so that I can bind to its value from the parent scope. Since the directive is from a third party I would like to do so without modifying the original code. In Angular 1.2.x, I used to be able to do this to decorate the existing directive with a new two-way binding.

app.config(function($provide) {
    $provide.decorator('mycustomDirective',['$delegate','$controller', function($delegate, $controller) {

        var directive = $delegate[0];
        angular.extend(directive.scope, { 
            othervar: "=" 
        }); 

        return $delegate;  
    }]);
});   

However, in Angular 1.3.x, extending the scope this way no longer seems to work. This is evident in this simplified plunkr: http://plnkr.co/edit/bwgwGm6IcoRfLjIL8K9X?p=info.

Changing the script tag to 1.3.x breaks the behavior. It seems all other properties of the directive can still be modified using $provide.decorator, but the scope in particular no longer seems to get updated.

Is this a bug? What changed in Angular 1.3.x to cause this specific behavior? And what is the recommended way of extending the scope of an existing directive (with an isolated scope)?

@jbedard
Copy link
Collaborator

jbedard commented Nov 20, 2014

This is caused by 56f09f0 - the scope info is now parsed once instead of once per link so it isn't picking up the changes.

I'm not sure if modifying properties of a directive is supported like this, but maybe the scope parsing can be lazy instead of done on initialize or something like that...

@sumigoma
Copy link
Author

Potential bug as described here.

Used workaround shown in this plunkr: http://plnkr.co/edit/CRxhX6?p=preview

@Narretz
Copy link
Contributor

Narretz commented Nov 21, 2014

@caitp Do you know if something like this was ever supported?

@caitp
Copy link
Contributor

caitp commented Nov 21, 2014

Well, we pre-compute isolate bindings during directive registration, rather than instantiation --- that was an optimization we made a while back (I think @jbedard was behind that one, although it might have been Igor). That's what would break it, though.

@jbedard
Copy link
Collaborator

jbedard commented Nov 21, 2014

Ya that was me (see comment above). Was this ever "supported" though? Modifying a directive definition seems little weird to me...

@btford
Copy link
Contributor

btford commented Nov 22, 2014

I also think this is a bit weird. I'm curious what your use-case is for this, @sumigoma. There might be a better general approach.

@pkozlowski-opensource
Copy link
Member

It looks fishy for me as well. It looks like an attempt to build on top of a directive (read: hack) that wasn't designed for extension. I guess directives that are designed for extension should expose their flex / extension points via a controller and don't require people to mess with a scope.

@sumigoma
Copy link
Author

Yes, it seems the directive I'm trying to use isn't particularly designed for extension, but I needed some way of exposing part of its internal state. I got the idea from here.

@yangchristian
Copy link

+1. My use case is around directive templates: I'm using a 3rd party directive and want to alter some standard markup in its template to work with other directives. Here's the concrete scenario since that last sentence was a bit abstract:

  • The 3rd party directive template has some simple <a> tags with an href binding, e.g. <a href="{{ href }}">
  • I want to supply a ui-router state, e.g. <a ui-sref="{{ state }}">
  • I used to decorate the directive like OP, now I'm using the .$$isolateBindings workaround

@arjunasuresh3
Copy link

+1

@iainjreid
Copy link

As @sumigoma noted, appending 'Directive' after your directive name allows you to then decorate that directive. I'm currently using this method in Angular 1.4.x.

And the Plunker that @sumigoma created has a great workaround to alter the directives scope: http://plnkr.co/edit/CRxhX6?p=preview.

Here's a brief example of how you could extend the scope easily...

var scopeExtension = {
  adsLinkHeader: '=',
  adsDataObject: '='
};

angular.forEach(scopeExtension, function (value, key) {
  $delegate[0].$$isolateBindings[key] = {
    attrName: key,
    mode: value,
    optional: true
  };
});

@Wlada
Copy link

Wlada commented Aug 23, 2015

+1

@newmesiss
Copy link

as I solve this if I need to execute a method in controlling father?
which is ativado with ng-click within the directive used

@nmccready
Copy link

This is a serious limitation please see the linked issue to ui-leaflet. This is one of the proposed ways that we are looking to support leaflet plugins. That way our directive can be extended to support other attribute directives with one isolated scope directive.

That way we avoid this ugly beast for every added plugin.

https://github.com/angular-ui/ui-leaflet/blob/master/src/directives/leaflet.js#L9-L21

Is this even considered a real bug yet?

@Narretz
Copy link
Contributor

Narretz commented Nov 5, 2015

I don't think it has been considered a bug yet, simply because the behavior was essentally undocumented and the use cases aren't that common.

@jbedard
Copy link
Collaborator

jbedard commented Nov 5, 2015

And it is still possible, it's just uglier then before. Well worth the performance improvement every time the linker runs (on an isolated directive)...

@yahyaKacem
Copy link

👍 @iainreid820, @sumigoma.

lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Dec 21, 2015
Allow directives to have interceptors that modify the directive `scope` property

Close: angular#10149
lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Dec 22, 2015
Allow directives to have interceptors that modify the directive `scope` property

Close: angular#10149
lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Dec 22, 2015
Allow directives to have interceptors that modify the directive `scope` property

Close: angular#10149
lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Dec 22, 2015
Allow directives to have interceptors that modify the directive `scope` property

Close: angular#10149
lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Dec 22, 2015
Allow directives to have interceptors that modify the directive `scope` property

Close: angular#10149
lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Dec 22, 2015
Allow directives to have decorators that modify the directive `scope` property

Close: angular#10149
lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Dec 31, 2015
Allow directives to have decorators that modify the directive `scope` property

Close: angular#10149
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests