Skip to content
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

Support for .component as well as .directive #221

Open
glendaviesnz opened this issue Jul 13, 2016 · 9 comments
Open

Support for .component as well as .directive #221

glendaviesnz opened this issue Jul 13, 2016 · 9 comments

Comments

@glendaviesnz
Copy link

Hi

Do you know if anyone is looking at adding support for passing in .components as well as .directives? Not a major issue not having this, but would be happy to look at adding it if nobody else is, and if you didn't see any issues with supporting it.

Thanks
Glen

@Stabzs
Copy link
Collaborator

Stabzs commented Jul 14, 2016

This wasn't necessarily on my radar, although I understand the reason for the change.

This would require a fair amount of change to the way directives (and in the future, components) are rendered since currently there are restrictions preventing isolate scope and element-based directives/components, both of which are reasons why components are currently incompatible with the directive bodyOutputType.

I think it makes sense to make the current rendering strategy more flexible instead of adding yet another bodyOutputType just to support components.

@glendaviesnz
Copy link
Author

ok, thanks - I will have a look and see what might be involved in this, and assess if it is worth the effort or not, and come back to you.

@glendaviesnz
Copy link
Author

I have had a look at this and it appears that the only way to tell if it is a component that has been passed in is to either check for the existence of $$bindings which .components do not have, or the default $ctrl controllerAs name. This second option is obviously not very reliable as this can be changed on a component, and can also be added to a directive.

Not sure how reliable/recommended it is to be relying on one of the angular private $$ variables for doing this check, but with a simple check for $$bindings it is possible to bypass the isolate scope check and A limit check for components and then to compile the template in the form
'<' + directiveName + ' directive-data="directiveData"></' + directiveName + '>'
for .components.

This seems to work ok. If you thought that this looked like a possible way to implement .component support without having to add another bodyOutputType as you mentioned let me know and I can put in a pull request with this modification added.

@glendaviesnz
Copy link
Author

ok - hold fire on looking at this - looks like directives don't always have $$bindings either - will do some more research on this.

@glendaviesnz
Copy link
Author

It looks like the easiest way to support .component is to only allow them to be passed along with the bodyOutputType = directive if the .component bindings param is set - at this stage this appears to be the only param that is supported by .component but not .directive.

Passing in a .component('mycomponent', {
bindings: { directiveData: '<' }
});
and rendering with
'<' + directiveName + ' directive-data="directiveData"></' + directiveName + '>'
if directiveDetails.bindings is set appears to work without too much modification of directive handling.

@Stabzs
Copy link
Collaborator

Stabzs commented Jul 19, 2016

Thank you for the detective work on this.

This looks plausible. I'd like to take a closer look as well. It's possible that changing the scope binding to be more flexible might be a nice touch as well, which would help alleviate a bit of this pain.

That said, the short path would be to potentially check for the presence of the binding if the restrict directive type is 'E', since components can only be element directives. That would help determine the type of html to stitch together to render.

I'll take a closer look at this when I have a chance. Thanks again for your work on this!

@glendaviesnz
Copy link
Author

There is a branch here https://github.com/glendaviesnz/AngularJS-Toaster/blob/component-support/toaster.js with the change I made to check this approach - happy to do some more work on this, add some tests, etc. and put in a pull request if it helps, but no worries if you want to just explore later yourself - just using .directive instead is working for me at the moment.

@AndreErb
Copy link

Do I understand this issue correctly, that AngularJS-Toaster currently doesn't work when used within components?

@Stabzs
Copy link
Collaborator

Stabzs commented Jan 18, 2019

@AndreErb no, that isn't correct. It just doesn't allow components to be passed to be dynamically rendered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants