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

feat(1.x): support routing to a directive #228

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

hannahhoward
Copy link

Implements #161

This is a first implementation. You would use it like so:

$router.config([
  { path: '/user/:name', component: 'user' }
]);

var DirectiveFactory = function() {
   return {
      template: "<div>{{user.myName}}<div>", // or templateUrl
      bindToController: {
         myName: "=name" // doesn't matter could be '@'
      },
      controller: UserController
   }
   function UserController() {
   }
}

// ideally it would be nice to set $routeConfig on the
// directive controller, but it's very hard to extract it
// from inside the factory function
DirectiveFactory.$routeConfig = [
    { path: "/subComponent", component: 'subComponent'}
]

angular.directive('user', DirectiveFactory)

Feedback welcome -- there are probably lots of ways to improve it.

Review on Reviewable

@eXaminator
Copy link

This is great, though I don't really like the $routeConfig on the factory. It would be much better on the Controller, though I can't really help on how to archive this.

I just tested it but I seem to have some problems with it.

  1. I don't seem to be able to access controller data from within the template...
  2. Can I use controllerAs with this?
  3. I get errors if I try to inject $scope, $element etc.

I noticed that you seem to access the template and just render it within the view. But this makes it impossible to write "normal" directives and just use them as components (and possibly vise versa). But a component in this case should just be a normal directive. It would be hard to explain and argue why this particular directive isn't allowed to use $scope or $element (and I haven't even tried what happens if I use a link or compile function).

Maybe it would be better to just render and compile it the old fashioned way. In your example you could just render <user name="my URL param"></user> inside the viewport and compile it. You might have to check the restrict property though to define how it should be rendered. This would also make it easier to apply css styles to a component as you could just use the tag name for selectors.

@hannahhoward
Copy link
Author

thanks for the feedback -- yea that's another approach and maybe makes better sense? (certainly solves problems like the $scope + $element injection). the only trick then is generating the logic for generating the view based on the restrict element. i.e. if the restrict is A does it become <ng-viewport user></ng-viewport>? or <ng-viewport><div user></div></ng-viewport>?

As for scope variables, like the way the router works with controllers, controllerAs syntax is used by default, so values should be set on this, and then it becomes controllerAs = nameOfComponent. Was this not working for you?

I am really stumped on how to put the $routeParams on the controller. According to @btford 's original proposal, the point of setting $routeParams as an attribute is so they can be culled ahead of time by for example a server side rendering program. I don't know how to do this without setting some fairly big constraints on how directives are used in Angular. Because at least currently, controller is an attribute on the return value of the directive factory. And the directive factory is dependency injected, which means it can't be run until the provider phase is complete (and theoretically the controller's definition could be affected by the dependencies injected). Anyway, just some thoughts.

@eXaminator
Copy link

I would not update the viewport element itself, it would be to dangerous as you would have to make sure to clean it up properly before adding a new component to it. So your <div user></div> version would make more sense and is probably a lot safer.

I missed the part about using the component name as alias. And I don't think it makes sense here. If you use a directive I would expect its controllerAs property (and all other properties) to work as normal. So this would probably be fixed by it self if you just render the directive as the template.

As for the params... I would keep it simple I guess. I think if you have something like /user/:username it should just produce something like <user username=":username"></user>. Obviously this does only work for simple strings, but what else could you logically get via a $routParam anyway? This might need to get some more attention once there is a way to add data via the router (like the data and resolve options from ui-router).

So I wouldn't even consider to inject $routeParams to directives as this would again break the possibility to just use the directive as, well, a directive within your HTML.

I still don't have any good advice on how to get the $routeConfig from the controller, so we might be stuck with your current solution. I'm also not quite sure if we would have access to the correct child $router within the controller of the directive if we don't load and inject it manually. I actually think it would be nice if I used a directive the old fashioned way (as HTML tag or attribute) and it could still register its own routes but not on its own child router, but on the router used by its closest parent component. I don't know if this would work as of now.

Example:
I have a directive users that defines some routes (probably via $router) like {path: '/detail/:username', component: 'userDetail'}. The template just renders a list of users and a viewport. When clicking a user its details should be rendered within the viewport.

Now I can call the users component via a route (let's say from within my app component) like {path: '/users', component: 'users'}. That's fine, users would just register its routes as children of the app router and we would end up with something like /users/detail/:username.

But it would be awesome if I could just go to the template of my app component in this case an just add <users></users> to it. In this case I would like the users component to attach its routes directly to the $router of app. This would have us end up with a route like /detail/:username which would still work.

Obviously there are some edge cases where this wouldn't be so straight forward, like what if I add more such directives and all add a default viewport. Of course it would be great if we could handle this somehow by matching the correct viewport to the correct routes, but I'm not sure if this would just be too much. But something like this would greatly increase the reusability of components as I wouldn't have to add routes just to use them.

Any thoughts on this?

@hannahhoward
Copy link
Author

That sounds super cool. The challenge is that right now, using a plain directive, the $router you'd get injected in the directive controller is not the parent scopes but rather the top level scope $router, unless I'm misunderstanding something in the Angular source code for compile.js & controller.js. Essentially when Angular sets up a controller using the $controller service, you have the option to pass it "locals" that overrides global dependency injections. That's how $scope and $element get injected in your directive's controller in the first place. Simultaneously, the router's source code uses this same "locals" property to override the $router that's injected into a components controller to be the child router for that component. I'm not exactly sure how one could get the right $router injected in to a directives controller using a plain directive, since the source for what locals go into a directive controller is in compiler.js in the angular main source code. So here we're faced with a dilemna that also applies to the strategy of putting the directive as an element inside of an ng-viewport -- either we use an element inside the ng-viewport, in which case we get all the benefits of angular's built in compiler and the way it handles normal directives but no localized $router/$routeParams, or we inject the controller manually as the PR currently does, in which case we can get a localized $router/$routeParams, but we're also essentially using directives in a totally different way then they're intended to be. It seems like at some point this has to be deferred to the Angular team, cause it seems like for an ideal solution (as well as putting $routeConfig on the directive's controller), there has to be some changes in the ng-source code, perhaps when they implement angular/angular.js#10007 .

@eXaminator
Copy link

I see, this indeed is a problem. Well, there would always be the possibility of binding the router via an attribute, but I guess that would be quite an ugly hack. But right now I can't think of any other way of doing this without going knee deep into angular itself. Maybe some way via the scope (though there would probably be some problems with isolated scopes).

But like I said, this would not fix the "multiple default viewports" problem.

@hannahhoward
Copy link
Author

it just occurred to me there are some additional issues raised with inserting the directive as an element inside the ng-viewport directive -- namely, the router wants to call the hooks on the controller if they exist -- namely canActivate, canDeactivate, activate and deactivate. That means the router needs to instantiate the controller itself. That's not compatible with just putting the directive inside a template. Meanwhile I'm not sure it's possible to inject $scope and $element on the controller -- though perhaps maybe with some additional work. It is possible to respect the directives controllerAs attribute, and I've gone ahead and made the PR do that, but using controllerAs syntax will be required.

@eXaminator
Copy link

You are absolutly right. We would need an alternative API for that I guess.

Here's a crazy idea: Wht if ngViewport defined a default controller itself. As we have the directives know about routing anyway (by having them declare routes), we could also have them require: ngViewport (maybe optional if one whishes).

The ngViewport controller could allow for all sorts of stuff.

  1. We could register our own controller as a subscriber for the hooks (ngViewport controller would then check if the necessary methods are availble on our directives controller).
  2. We could even get the correct $router services this way.

Maybe there are more advantages... there are probably some disadvantages. Any thoughts?

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

Successfully merging this pull request may close these issues.

2 participants