$state injection into resolve function gives old state

[kevinsbennett]

When passing in $state into a resolve function, it seems to provide the old state, not the new state that we're resolving for.

resolve: { model: ["$state", function($state){ //$state.current gives info for old state here }] }

This is a problem as we can't access custom state data in the resolve function.

[ksperling]

It's a little tricky... $state is a service, so there really is only one $state.

We also can't really update $state.current before the resolve process starts, because (1) it's asynchronous, so the state hasn't changed yet, and (2) it could fail.

I'm thinking a nice way to solve this would be to have a special injectable like '$transition' which would be the transition object for the current transition, with properties like 'to', 'from', 'toParams', 'fromParams', ...

[nateabele]

Yeah, :thumbsup:. The $transition service could be mirrored to that encapsulation inside transitionTo() that we talked about.

[ksperling]

Yeah, i think the 3rd parameter to transitionTo becomes the transition object

[ghost]

This seems like a design flaw that does make rather common use cases virtually impossible. If my route includes a page parameter, and that route's resolve map needs to pull up that "page" of data from my API, it has no way of obtaining this information without parsing location.hash itself--at which point I might as well not be using any framework's router.

I understand the timing issues involved, but this strikes me as a fairly huge hole in the system.

[laurelnaiad]

@oncomeme -- can't you access the parameter directly in your resolve, instead of seeking it out in the $state service? Or am I misunderstanding?

[romario333]

@oncomeme - as @stu-salsbury mentioned, you can inject $stateParams into the resolve function. That seems to be working just ok.

[ghost]

It works. It's also really awkward and makes it impossible to define a truly standalone service as it has to be baby fed by everything that wants to use it.

[nateabele]

I believe the phrase is "patches are welcome".

[ksperling]

URL parameters of a state are parameters of a state. As such I would consider it bad design (essentially a layering violation) for a service to access these directly. A state-level 'resolve' that passes correct parameters to the correct service is the way to go.

[ProLoser]

Be careful about naming conventions as $transition is used inside of ui-bootstrap and may get picked up by animation code in the core.

[ksperling]

@ProLoser I've been thinking there should be a naming convention for "local" magical values that are injectable, that makes them easily distinguishable from global services. Maybe $transition$, i.e. starting and ending with $.

[nateabele]

@ksperling I've been thinking, what if, instead of a separate service, we just overwrote $state.transition and gave it a more robust API (i.e. never make it null, and add some getters for status & to/from)?

[timkindberg]

@nateabele that makes sense to me. The $transition service idea felt like it could get more complicated for users. Also keeps the transition object "namespaced" on $state, so we'll avoid the naming convention issues @ProLoser mentioned (which obviously is your goal). I like it.

[ksperling]

@nateabele Not sure that really solves it... I think the code that runs during the processing of a transition always has to see "new" values relating to that transition, where as everything else shouldn't see that data until the transition succeeds. For example if while the resolve is running the user does something else in the UI, that code needs to see the previous values.

Also, if a second transition gets started which supersedes one that is currently running, the values for the second one should be separate from what the previous one is looking at, otherwise it will be pretty easy to get unpredictable stuff happening.

I think having a special injectable like $transition$ shouldn't be too confusing -- for starters I wouldn't call it a 'service' and wouldn't make it available globally from $injector, so that there is no expectation for it to behave like a normal service.

[timkindberg]

@ksperling hmmm, well sounds like $state.transition is too hard to implement (or just not feasible). I'm not sure how I feel about the $transition$, but I suppose if that's how it needs to be done then that's ok. What about $$transition instead? Or is the double $$ already used conventionally to mean something else?

[nateabele]

@ksperling:

I think the code that runs during the processing of a transition always has to see "new" values relating to that transition

Why not let it see everything? Here's a proposed object structure:

$state.transition = angular.extend($promise, {
    to: targetState,
    from: currentState,
    params: {
        to: { ...all params for target state... },
        from: { ...all params for current state...}
    }
});

The $promise bit is to signify that the object itself can still be used as a promise, i.e. by doing $state.transition.$then(...).

Am I missing anything here?

[ksperling]

@nateabele Imagine you do something like

$state.transitionTo('foo', { id: 1 });
// and shortly after, before the first transition completes
$state.transitionTo('foo', { id: 2 });

Any resolve functions, callbacks etc that happen as part of settings up that first transition need to see id==1. Anything that happens as part of the second transition needs to see id == 2. Additionally, any global service that happens to look at e.g. $stateParams before either of these two transitions completes shouldn't see either id==1 or id==2, but should see whatever was in $stateParams before -- essentially a transition that's in progress is analogous to an uncommitted database transaction; nobody else should see those values (yet).

So what I'm saying is that a transition has this private information (which is currently just $stateParams, but I'd like to have a transition object with 'state', 'params', 'fromState', 'fromParams'), which only becomes (or gets copied into) globally accessible $state once the transition succeeds.

To make this private information available, we're using the 'locals' parameter of $injector.invoke. I was arguing that using an identifier like $transition$ that's not also used as a global service name might would be less confusing than what we've currently got with $stateParams, which magically behaves different depending on context.

I agree that we can also expose the latest active transition object as $state.transition, but that doesn't replace the need for a special injectable like $transition$ to make sure that transitions happening in parallel see their own isolated state.

[nateabele]

@ksperling Okay, so essentially the transition information needs to be scoped in the same way as $stateParams is currently. Did I understand that correctly?

Also, as far as names go, I think a name like $stateTransition solves all problems, doesn't look weird, and establishes a logical pattern.

[ksperling]

@nateabele yes on the scoping.

I think $stateTransition is OK, I just think it might be good to avoid the confusion of global vs scoped behaviour that we currently have with $stateParams. In that sense I consider $transition$ looking a little weird to be a feature, because we can then say "it looks weird because it's not a global service and is only available in certain contexts".

I think we could even go so far as removing $stateParams as a global service, and allowing it to be access as $state.params in the global context (only completed transitions) and as $params$ for controller / resolve injection.

If we go with $stateParams and $stateTransition it will appear more obvious at first, but then we'll get the issues like we've seen where people inject it into a service and it doesn't behave how they expect.

[timkindberg]

@ksperling I actually agree now. It is a new type of injectable that devs will NOT be familiar with. So having it look special will help keep devs on high alert. I wasn't even aware that things could be injected at a very specific level until recently. So I like it. The extra $ means its an injectable that has been scoped.

[a8m]

@nateabele I had $stateTransition property to state obj, now we can get an access to 'toState' property in the resolve function..

[petebacondarwin]

How about:

.value('$stateChange', { toState: {}, toParams: {}, fromState: {}, fromParams: {} })

.run(['$rootScope', '$stateChange', function($rootScope, $stateChange) {
  function updateTransition(event, toState, toParams, fromState, fromParams) {
    $stateChange.toState = toState;
    $stateChange.toParams = toParams;
    $stateChange.fromState = fromState;
    $stateChange.fromParams = fromParams;
  }

  // Ensure state transition info is available to be injected during a state transition
  $rootScope.$on('$stateChangeStart', updateTransition);
  $rootScope.$on('$stateChangeError', updateTransition);
  $rootScope.$on('$stateChangeSuccess', updateTransition);
}])

[petebacondarwin]

Related to #578

[bvaughn]

+1

I've had to work around this in my current app by observing $stateChangeStart events and manually storing an targetState attribute on the $state service. (Alternately I could store it in a custom value, as @petebacondarwin has suggested.)

This is an important feature in order for a parent state to intercept a route and set a default parameter if missing.

[nateabele]

This has been superseded by #1257. Please follow that issue for updates. Thanks!