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

Idea: Configuring routes without the need for a dependency to call a module #486

Closed
mboudreau opened this issue Oct 8, 2013 · 5 comments · Fixed by #492
Closed

Idea: Configuring routes without the need for a dependency to call a module #486

mboudreau opened this issue Oct 8, 2013 · 5 comments · Fixed by #492
Labels

Comments

@mboudreau
Copy link

We're currently writing a fairly large web application with Angular 1.2-rc2 and UI-Router. We've come into contact with an issue that is a bit problematic for using ui-router. Most of our routes have their own modules because they contain quick a bit of functionality and it's easier to separate concerns, but the problem we've encountered is when you have a parent module that's dependent on a child module, where in this child module it's state parent is the original module. This would cause an error because the parent state hasn't been instantiated yet because it's still waiting for it's dependencies.

Here's an example of what I mean, this is how ui-router should function (as per the example in the docs):

angular.module('spaces', [ 'ui.router'])
.config(function ($stateProvider)
    {
        $stateProvider
            .state('spaces', {
                url: '/spaces'
            });
    });

angular.module('spaces.details', ['spaces', 'ui.router'])
    .config(function ($stateProvider)
    {
        $stateProvider.state('spaces.details',
            {
                url: '/{id:[0-9]{1,12}}'
            });
    });

The problem here is that the config of 'spaces.details' will never run unless something is dependent on it. In this app, you get to 'spaces.details' from 'spaces' which isn't a dependency per say, so our only way to actually get the config function of 'spaces.details' to run is to add that dependency to 'spaces' since it would seem like the logical place, like this:

angular.module('spaces', ['spaces.details',  'ui.router'])
.config(function ($stateProvider)
    {
        $stateProvider
            .state('spaces', {
                url: '/spaces'
            });
    });

angular.module('spaces.details', [ 'ui.router'])
    .config(function ($stateProvider)
    {
        $stateProvider.state('spaces.details',
            {
                url: '/{id:[0-9]{1,12}}'
            });
    });

That looks all good, but now there will be an error running it saying that 'spaces.details' route doesn't have a parent because the 'spaces.details' config function ran before it's parent ('spaces'). This circular problem is really annoying. The solution we have for now is to have all modules using ui-router to be a dependency of our main app.js file, which in turn runs all configs for every module and sets the routing properly. Furthermore, I find it annoying that the dependency model is backwards, in this case, 'spaces.details' is dependent on 'spaces' and it's data, not the other way around, which would cause confusion.

My proposed solution to this is to add a new function that can be attached after the module definition that specifies the routes and is ran the second the module is defined. Something along these lines:

angular.module('spaces', [ 'ui.router'])
    .state('spaces', {
            url: '/spaces',
            controller: 'SpacesCtrl',
            templateUrl: 'spaces/spaces.tpl.html'
        });

angular.module('spaces.details', ['spaces', 'ui.router'])
    .state('spaces.details', {
            url: '/{id:[0-9]{1,12}}',
            controller: 'SpacesDetailsCtrl',
            templateUrl: 'spaces/spaces-details.tpl.html'
        });

And the order in which the state is called is pretty much a non issue as long as the code either adds a placeholder until the parent is defined or that if the parent is missing that it would find it before continuing the route definition.

That's about it, sorry about it being long, I was just trying to think out loud. I'd love to hear about opinions and suggestions on this. If nobody has an objection, I might try to get this working myself and do a PR when it's complete.

@nateabele
Copy link
Contributor

Yeah, the idea of compiling state configurations just-in-time (i.e. as you go from config to runtime) has been brought up and would solve this problem.

It's definitely something we're interested in doing, we just haven't had the time or available resources. If you're interested in hacking on this, let me know and I can point you in the right direction.

@mboudreau
Copy link
Author

Point away :)
On Oct 9, 2013 12:21 PM, "Nate Abele" notifications@github.com wrote:

Yeah, the idea of compiling state configurations just-in-time (i.e. as you
go from config to runtime) has been brought up and would solve this problem.

It's definitely something we're interested in doing, we just haven't had
the time or available resources. If you're interested in hacking on this,
let me know and I can point you in the right direction.


Reply to this email directly or view it on GitHubhttps://github.com//issues/486#issuecomment-25940670
.

@nateabele
Copy link
Contributor

@mboudreau Word.

So, here's the entry point for state configuration: https://github.com/angular-ui/ui-router/blob/master/src/state.js#L208-L215

Either you could queue states there based on the level of depth of their parent (which would have to be normalized, since there are currently 3 ways to specify parents), or you could do some sort of dependency tracking.

Then, at at top of $get(), you would actually compile them all. That's all I can think of off-hand. Follow up with any other questions you might have.

@rixo
Copy link

rixo commented Nov 20, 2015

(@nateabele I don't know if I'm supposed to post here or open a new issue, so sorry if I missed)

I suppose this is an edge case (or blatantly unsupported), but the following is still broken in current master:

.state('app.organization', {
    parent: 'app.settings'
    // ...
})

Depending on file inclusion order, it can crash as described in #488.

I was able to fix it for myself by changing the order in this hard to read statement:

  function registerState(state) {
    // ...

    // Get parent name
    var parentName = (name.indexOf('.') !== -1) ? name.substring(0, name.lastIndexOf('.'))
        : (isString(state.parent)) ? state.parent
        : (isObject(state.parent) && isString(state.parent.name)) ? state.parent.name
        : '';

    // ...
  }

To this:

  function registerState(state) {
    // ...

    // Get parent name
    var parentName = (isString(state.parent)) ? state.parent
        : (isObject(state.parent) && isString(state.parent.name)) ? state.parent.name
        : (name.indexOf('.') !== -1) ? name.substring(0, name.lastIndexOf('.'))
        : '';

    // ...
  }

I didn't run the tests though.

@nateabele
Copy link
Contributor

@rixo Yes, what you're doing is explicitly unsupported. Defining a state name as app.organization (i.e. using dot syntax) means the parent state is app. A state can't have two parents.

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

Successfully merging a pull request may close this issue.

3 participants