add feature to clearAll and (re)initialize providers to support on-the-fly routing changes

[laurelnaiad]

Background: it would be beneficial to be able to modify routing during runtime. For example, when a user is logged into a system, the routes should be different than when the user is not logged in. In another example, imagine a CMS. The admin user can add a blog feature to the CMS. In order to preview the blog, a new route should be defined so that the blog can be seen. Other cases exist where the ability to define and undefine routes would be useful.

I looked at being able to modify routes/states on the fly in ui-router. There are some challenges.

(1) It doesn't expose the route definition functions in the services. This is easily surmountable. In fact, this pull request contains a test spec that uses a service to access the providers. So no big deal there, but it would be nice fold this into ui-router at some point.

(2) The ui.state and ui.compat modules do not keep track of the rules that are defined for them in urlRouter. The challenge here is that if you want to pick out a state or a "route when" that you want to modify or remove, it would be unwieldy at best to edit the rules in the urlRouter accordingly. It would be great if those two packages could track which rules go with which states and routes so that they could be modified or removed later.

(3) Related to challenge (2), the "otherwise" rule is appended to the end of the rules array when the urlRouter service starts. This complicates one potential solution to (2), which would be to track the index position of the rule that relates to a state or when route.

So, in the long run, I'd like to solve these three challenges. I hope that such a feature is welcome in ui-router and I'd appreciate some feedback on it.

In the short run, this pull request adds clearAll and init methods to the providers so that an app can wipe out the routes and redefine them. Because the otherwise rule needs to remain last, the URL provider is very slightly modified to use init() when starting the service.

The test spec uses the technique of exposing the providers to a service so that they can be used at runtime.

Feedback is welcome. I hope clearAll/init can be supported so that people will be able to use my larger project which is to manage both routes and a generically applicable navigational UI directive in a datadriven manner. More on that later.

Thanks!

[laurelnaiad]

One note: I'd like to do a pull request to expose the configuration functions from the services in the same manner that the test specs "stubby" provider does... so if there's a yay or nay on that idea, please let me know.

[jeme]

There is a huge problem that this doesn't seem to address the concerns of at all: #95 (comment)

Modifying or adding changes to routes on the fly, what happens when you go to such a route and hits refresh, the case happens for many cases this will now suddenly not exist and the user would be thrown to the otherwise handler.

So we need to figure out how to solve that issue. Imagine your login case... the user goes to the page, logs in, goes to a private page and hits refresh... and gets thrown out of the private area into the front page or worse, a "404"...

As of your chalenges:

1. Shouln't be an issue as you your self mentions, but we need to make damn sure that programmers don't think they can just add routes/states here and there, and it will somehow get automatically discovered on refresh as well... It will create broken links for sure unless they remember that the server-side is also dynamically changing the initial configuration.
2. I think ui.compat should be left out of this entirely, this is a module made for backwards compatibility with the angular core routing scheme, since this scheme doesn't support this sort of thing, then there is no need at all to align things. (For new applications, ui.compat shouln't be used)
3. That sounds like the evaluation of rules isn't quite done in the same manner as in angular and angular routing... I would encourage to simplify it towards those implementations in that case, as they don't care when the otherwise route is added, as it just never matches anything, but is the one returned in the end if no other picks up the path...

In the core essense, this is how it works: (pseudo)

var routes = {};
routes[null]= 'oterwise route';
routes['/path'] = 'path route';

forEach(routes, (r, k) => {
  if(k && k.matches(path))
    goto r.
})
goto routes[null];

To solve a data driven approach, I think it would be better to consider some way of running initialization POST start-up instead, to clearly indicate the dependency between the server and the client rather than just allowing arbitrarily to call $state.state('x', {}); here and there and then ask questions when the user can't directly navigate to /route/state/x...

Something like:

$stateProvider
    // One shutoff initialization or allow multuple?
    // state provider will be given as a local.... Initializers is run up front when the $state 
    // service is requested. and I don't think we need any special case handling of 
    // "initialized" or not as the get method should only be called ones by angular.
    .initializer(['$stateProvider', '$http', function($stateProvider, $http) {
        var promise = $http.get('/routeconfig');
        //Hmmm.... We now have a promise, so that is an issue we need to figure out.
        return promise.then(function(data) { 
            forEach(data, (state, key) {
                $stateProvider(key, state);
                //hmmm... what about controllers, dynamically load a javascript or something?...
            });
        });
    });

I am fairly sure that if people begin to do things like this:

$stateProvider.state('root', {
    url: '/home',
    template: 'bla',
    controller: function($state) {
        $state.state('root.child', {
                url: '/child',
                template: 'fubar',
        });
    }
});

I am 100% sure that they will ask question to why it goes to otherwise when they type in /home/child in the browser.

All-in-all... I think this gets extremely complicated and complex.

[laurelnaiad]

@jeme thanks for taking the time to think about this. It's not in any way meant to address issue #95 , so I'll leave that part alone.

Modifying or adding changes to routes on the fly, what happens when you go to such a route and hits refresh, the case happens for many cases this will now suddenly not exist and the user would be thrown to the otherwise handler.

So we need to figure out how to solve that issue. Imagine your login case... the user goes to the page, logs in, goes to a private page and hits refresh... and gets thrown out of the private area into the front page or worse, a "404"...

If the login succeeds the valid routes will be available. My demo is just a dumb demo -- I don't think I'd actually remove the login route once the user logs in (in fact I think I put a comment to that effect in the test case.

So: the front page would still be a valid route. The private area is the area that indeed would become a valid route when the login succeeds. All of the valid routes would be valid. This is the app's responsibility. It almost sounds like you're saying that there is a risk here because an application might not define a route that should be valid -- which is always a possibility whether you define routes at runtime or config time.

As of your chalenges:

(1) (modifying routes at runtime) Shouln't be an issue as you your self mentions, but we need to make damn sure that programmers don't think they can just add routes/states here and there, and it will somehow get automatically discovered on refresh as well... It will create broken links for sure unless they remember that the server-side is also dynamically changing the initial configuration.

Cool -- as I said, this is a nice-to-have... it's already possibile but requires a provider-enabled service in order to get at the functionality. I don't have any particular need for challenge (1) to be addressed, it would just be cleaner not to need it.

(2) I think ui.compat should be left out of this entirely, this is a module made for backwards compatibility with the angular core routing scheme, since this scheme doesn't support this sort of thing, then there is no need at all to align things. (For new applications, ui.compat shouln't be used)

I agree -- I almost left it alone for that reason. I can revert those changes and do another pull request.

(3) That sounds like the evaluation of rules isn't quite done in the same manner as in angular and angular routing... I would encourage to simplify it towards those implementations in that case, as they don't care when the otherwise route is added, as it just never matches anything, but is the one returned in the end if no other picks up the path...

This is an issue outside of the presence of init and clearAll. The "otherwise" rule doesn't need to be merged into the rules array. Where now the urlRouter says

    function ($location,   $rootScope,   $injector) {
      if (otherwise) rules.push(otherwise);

      // TODO: Optimize groups of rules with non-empty prefix into some sort of decision tree
      function update() {
        var n=rules.length, i, handled;
        for (i=0; i<n; i++) {
          handled = rules[i]($injector, $location);
          if (handled) {
            if (isString(handled)) $location.replace().url(handled);
            break;
          }
        }
      }

      $rootScope.$on('$locationChangeSuccess', update);
      return {};
    }];

It might be better to say:

    [        '$location', '$rootScope', '$injector',
    function ($location,   $rootScope,   $injector) {
      //NOTE: no longer appends otherwise rule to rules array

      // TODO: Optimize groups of rules with non-empty prefix into some sort of decision tree
      function update() {
        var n=rules.length, i, handled;
        for (i=0; i<n; i++) {
          handled = rules[i]($injector, $location);
          if (handled) {
            if (isString(handled)) $location.replace().url(handled);
            break;
          }
        }
        if (!handled && otherwise) {
          handled = otherwise($injector, $location);
          if (handled) {
            if (isString(handled)) $location.replace().url(handled);
            break;
          }
        }
      }

      $rootScope.$on('$locationChangeSuccess', update);
      return {};
    }];

I didn't want to stir the pot any more than absolutely necessary with this change request so I didn't alter the algorithm, but this would be getting one step closer to being able to alter the routing table on the fly instead of wiping it out and rewriting it.

I hear your concerns about fragility. Nobody wants fragile, but app developers do need to be responsible for having valid routes at all times regardless of when in their code they set them up (runtime or config time).

When the CMS admin adds a blog feature, they're in runtime. As it stands now, init and clearAll are only exposed in the providers. I'm fine with leaving it that way because it means someone needs to go through some hoops to access them (or any other route changes) at run time. I guess this follows the same principle as angular core does. So really, I'd be prepared to take challenge #1 off the table entirely, in favor of having the route editing features fleshed out within the providers. Even that is icing on the cake -- routing changes should be rare enough that a full rewrite of the table isn't a big deal. Accessing the providers at runtime can be regarded as "black diamond" usage. But having init and clearAll in the providers is just that tiny change that makes it possible to rewrite the routing table when you really need to, and that's all I'm trying for with this request.

To solve a data driven approach, I think it would be better to consider some way of running initialization POST start-up instead, to clearly indicate the dependency between the server and the client rather than just allowing arbitrarily to call $state.state('x', {}); here and there and then ask questions when the user can't directly navigate to /route/state/x...

That wouldn't mean data-driven, that would mean delayed -- data can change. By not putting the api in the service, it should prevent anyone from accidentally discovering the api at runtime.

All-in-all... I think this gets extremely complicated and complex.

yup...and I hope ui-router can be used in complex apps! A flexible and yet powerful CMS is not a walk in the park :)

In all seriousness, though, I think that making routing editable within the providers (not ui.compat, though, and not in the services) would be the happy medium. Most people don't need it and those that do would need to do a little bit of fancy footwork to get to it at runtime, which should keep them from tripping over the feature in the way your example demonstrates.

[laurelnaiad]

BTW: the only thing "init" actually does is append the "otherwise" rule to the rules array, so if the code I put in the previous comment were the actual code, init wouldn't even be needed. Just clearAll.

[timkindberg]

I can see Stu's point of view here. The reason ui-router was created in the
first place was because the core router was too simple. I think the more
powerful it is, the better. I also see @jeme's point of view, in that it
will create bugs for people who use a feature like this unwittingly. I
would vie a bit more on the side of power; at the cost of responsibility.

The reason I even originally became involved in helping with ui-router is
because I was evaluating angular against ember to decide on a js framework
for my next project. Ember has an incredible routing solution, but angular
does not, it was too simple. Unfortunately, my next project has still not
started so I've really not even begun to use angular or ui-router in any
real world sense (hence my lack of participation of late), but I'm still
quite attune to how I'd like it to work once I start to use it.

Now I feel that ui-router is edging past Ember's router in terms of
capability. I think we need more of these deep capabilities to keep it in
the lead and keep attracting the devs who have these sorts of
'out-of-the-box' routing challenges.

[jeme]

That wouldn't mean data-driven, that would mean delayed -- data can change. By not putting the api in the service, it should prevent anyone from accidentally discovering the api at runtime.

The point was to start with exposing something that allowed you to let the server control the route configuration through a JSON object or something else. This is an important step as without this, it gets difficult to exchange route information with the server... the delayed part is merely convenience as it lets you use the $http service.

Taken your blog feature... This is what needs to happen:

• Admin adds blog feature
• Client sends a request to the Server that it should add the feature
• Server stores the information and adds a blog route to its "client route config"
• Client updates its route configuration. (first solution, simple reinitialize)

And I think this approach will make it easier to make sure that the "routes" are up to date, because we would instruct the programmer to call serverService.AddFeature(); $state.refreshConfiguration(); rather than doing the plumbing around what happens him self...

Hitting the refresh would obviously also work, because then the new route would exist in the initial initialization response from the server.

And this approach in the hopes that it would make it clear, that the server is the drive in this.

[laurelnaiad]

@jeme, yes, I think you nailed the workflow for the blog use case. But there is no need to delay for the http service... there doesn't need to be any delay. As soon as the server knows of a route, the client can pick up that route at runtime.

Another use case that I have neglected to mention is template versioning. When a template is updated, the templateUrl should change from /templates/myEverEvolvingTemplate?v=22 to /templates/myEverEvolvingTemplate?v=23.

@timkindberg -- I told myself I wasn't going to mention that other JS framework in an angular context anymore :) . I do hope that ui-router can continue to grow as the power tool in the box for angular. I also understand that it takes a while for folks to get comfortable with changes like this. Hence trying to withdraw the notion of putting any of this api on the services -- nobody's going to accidentally write a service that exposes the provider to themselves.

[jeme]

But there is no need to delay for the http service... it's because data changes at runtime.

That doesn't matter, because you wan't the same code to run regardless of standing in the middle of an application at runtime where you just asked the routing system to update it's configuration, or if you have just ventured into that app from a freshly started browser...

[laurelnaiad]

@jeme wrote:

That doesn't matter, because you wan't the same code to run regardless of standing in the middle of an application at runtime where you just asked the routing system to update it's configuration, or if you have just ventured into that app from a freshly started browser...

I think I agree with that but I'm not sure I understand it.

I know that I want the routing to always be up to date, and so any delays in getting it up to date should be minimized. Any delay which means that the client lacks a needed route is too much of a delay. That's about as simply as I can state it.

Are we discussing whether or not the same code will work at runtime that works at config time? If so, then I can assure you that the wrapper service in my test case can configure the routes in the module.run function before any routes are needed, and thus there is no need for anything to happen in the config function. That works with the ui-router as it stands today in angular-ui/ui-router without this patch. This patch just lets you wipe out the routes when you need to.

[jeme]

I know that I want the routing to always be up to date, and so any delays in getting it up to date should be minimized. Any delay which means that the client lacks a needed route is too much of a delay. That's about as simply as I can state it.

The delay is a functional delay, in terms of delaying it to after the Initialization phase is over and services become available, something you will have to wait for regardless of what approach you take because nothing happens until that phase is over anyways. (Run method isn't executing until after all config methods has been executed either)

Obviously compared to static routes, it adds that extra server round-trip, but there is only one way around that, and that is to deliver a static configuration that is dynamically generated by the server (which is an ugly approach IMO). And hearing that you use the run method to do things, I'm sure you already have that extra round-trip.

[laurelnaiad]

Yes, I concur. The good news is that the amount of JSON to download would be slightly fewer bytes than the bytes required to do it statically. Waiting until getting to config or run (which one really doesn't make much difference) to do download it does lengthen the pipeline a tad, but there are plenty of other dependencies to download so it's not like the browser is just going to be sitting there twiddling its thumbs. I suppose I could use requirejs to download an initial JSON object in order to get a headstart on the download if that becomes a problem, but I think that might be splitting hairs. All of this is getting a little far afield from wanting to have an accessible function that clears the routes and states! I just want to keep it clear that I'm not asking for all of my problems and challenges to be solved -- just that one function!!!

P.S. I just realized that github is pushing future commits on my branch into the pull request. I didn't know it did that. I hope that's not bad of me. I wanted to push the alternative to my branch so you guys could see it without the init function (which I agree shouldn't be needed if the urlRouter uses otherwise as rule seperate from the others).

[laurelnaiad]

Sorry -- one more thought... I'm not trying to set the world on fire here (at least not with this issue)... I just need a function that reinitializes the routes and states.

I'd like to take the concept of putting any of these functions in the services off the table forever, since it is not necessary -- the service in the submitted test case is in fact the service I'm using to do what I need to do and I won't ask for it to be folded into ui-router. Your points about accidental misuse and my recollection that a decision at the angular-core-level to make it difficult to access providers at runtime for that very reason convinced me that services are on their own to bind to providers within their own providers.

I'd also like to take the stuff about editing the routing table (as opposed to reinitializing the routing table) out of the conversation for now, since it is obviously a much bigger deal in terms of complexity, and it seems to be a distraction from the request here (which is to allow reinitializing the routes/states in the providers). I'd be super happy to discuss and/or work on that feature later because I think it would be spectacular. I've opened a separate issue for talk of editing: #126 . Can we take that subject over to that issue and focus this one on reinitialization? I regret that my use case dragged us down that path within the scope of this issue.