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

Dynamic (sync) resolvable added in onEnter hook not available in onSuccess #3544

Closed
1 of 3 tasks
adamreisnz opened this issue Oct 1, 2017 · 7 comments
Closed
1 of 3 tasks

Comments

@adamreisnz
Copy link

adamreisnz commented Oct 1, 2017

This is a:

  • Bug Report
  • Feature Request
  • General Query

My version of UI-Router is: 1.x

Bug Report

Current Behavior:

http://plnkr.co/edit/u0JKQCIabRCixHJLmBbB?p=preview

See console log. The app errors with:

Error: Resolvable async .get() not complete:"bar"

Expected Behavior:

For bar to be available in the onSuccess hook, just like foo, because it's not async and there is no reason why it should not be available/complete.

Expected behaviour is for it to work, just as it works in the onBefore hook.

(FYI the reason I need it in the onEnter hook is because I'm relying on another global resolvable to create the new resolvable, which is only available in the onEnter hook.)

@adamreisnz
Copy link
Author

@christopherthielen this issue is quite a big blocker for us, could you please have a look at what's happening or maybe provide some suggestions for a work around?

@adamreisnz
Copy link
Author

In fact, it's also happening in the onBefore hook when browsing away from the state, and then going back to it. The addResolvable is called fine, but then in the subsequent onSuccess the resolvable is not available in the injector.

@adamreisnz
Copy link
Author

adamreisnz commented Nov 19, 2017

You also reproduce this in the same hook, by defining the resolvable (sync) and calling the injector on the same transition right after; that shouldn't error right?

const module = {foo: 'bar'};
transition.addResolvable({
      token: 'module',
      resolveFn: () => module,
    });
transition.injector().get('module');

==> Error: Resolvable async .get() not complete:"module"

@adamreisnz
Copy link
Author

adamreisnz commented Nov 19, 2017

When using {when: 'EAGER'} policy, the module is available in onSuccess and doesn't error. However, according to #3060 (comment), LAZY resolves should also be available in onSuccess hooks.

@christopherthielen
Copy link
Contributor

christopherthielen commented Dec 21, 2017

This does look like a bug. The added resolve should be available in any (currently unentered) child states and in onSuccess. I have a unit test for something similar but it's not for this specific scenario.


A few things to note:

  1. You can add a resolvable with pre-resolved data (not async) using the data property:

add pre-resolved values

const module = {foo: 'bar'};
transition.addResolvable({
      token: 'module',
      resolveFn: () => module,
      data: module,
    });
transition.injector().get('module');
  1. The Injector has a getAsync() method which will return a promise for the resolve.

using async/await

    const injector = transition.injector();
    const foo = await injector.getAsync('foo');
    console.log(foo);
    const bar = await injector.getAsync('bar');
    console.log(bar);

using promises

    const injector = transition.injector();
    Promise.all([injector.getAsync('foo'), injector.getAsync('bar')])
      .then(results => { console.log(results[0]); console.log(results[1]) })
  1. Resolves are assumed to be async by default.

by defining the resolvable (sync) and calling the injector on the same transition right after; that shouldn't error right?

You can't do this unless you explicitly provide the data property to create a sync resolvable.

  1. EAGER resolves are fetched when the transition starts. LAZY resolves are fetched when the state they belong to is entered.

By adding the resolvable in bar's onEnter, there is no further triggers to process the newly added resolvable (all states have been entered already). This is essentially where the bug lies. To prove the point, if we added another nested state foo.bar then the resolve is indeed processed and ready in onSuccess (also change the criteria objects from to: 'foo' to entering: 'foo'

@adamreisnz
Copy link
Author

adamreisnz commented Dec 22, 2017

Thanks for looking into it and finding the bug, and also for pointing out data instead of resolveFn. I've tried to change resolveFn to data in the (plunkr)[http://plnkr.co/edit/u0JKQCIabRCixHJLmBbB?p=preview] but that errors with:

image

This is probably why I ended up not using it, because I get the same error in my app when I change resolveFn to data. Unless I'm expected to use both? But that doesn't make sense to me.

Not sure if this is related to the bug you fixed or not.

@christopherthielen
Copy link
Contributor

Unless I'm expected to use both? But that doesn't make sense to me.

Although it doesn't make much sense, use both. Maybe we can fix this in a future release.

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

No branches or pull requests

2 participants