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

add plain value support to task #220

Merged
merged 1 commit into from Oct 16, 2019
Merged

Conversation

xaviergonz
Copy link
Contributor

Like the name, make task support plain values (like await does)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 93.625% when pulling ab46fcc on task-supports-plain-values into 4927162 on master.

@benjamingr
Copy link
Member

That sounds reasonable to me - shouldn't a test be added?

@xaviergonz
Copy link
Contributor Author

I enhanced one of the existing tests to show that it works

@xaviergonz xaviergonz merged commit b9ee8fa into master Oct 16, 2019
@akphi
Copy link

akphi commented Oct 16, 2019

wow, I was about to file this as an issue. I was super hyped and changed all of my flow to actionAsync but for actions that do not return a promise (e.g. await is only called in one branch of an if/else) error would be thrown.

Hope this address that issue. Could you guys create a minor release for this? @urugator @mweststrate

Also, great work @xaviergonz. Thank you so much!

@xaviergonz
Copy link
Contributor Author

@blacksteed232 was the error you were getting about task not supporting a non promise or something else?

@akphi
Copy link

akphi commented Oct 16, 2019

@xaviergonz I have a function like this

@actionAsync
async doSomething(): Promise<void> {
    if (this.canDoIt()) {
        await task(/* some promise */);
    }
}

Funny thing is when I call this function once it’s fine but if I call it twice or 2 functions that look like this, it would throw

"actionAsync" context not present or invalid ...

Now if instead, I call

task(doSomething())
task(doSomething())

It would be okay, but that's obviously not what we want, we want to use task() inside of @actionAsync only.

Also if I modify my doSomething to

@actionAsync
async doSomething(): Promise<void> {
    if (this.canDoIt()) {
        await task(/* some promise */);
    } else {
        await task(/* some other promise */);
    }
}

It will work. So I look at your change and at least the title of this PR and I thought that’s it, we can wait for value as well. Am I right here, or my issue is completely unrelated?

Sorry if my markdown is screwed up, im at work so typing this from a phone :)

@xaviergonz
Copy link
Contributor Author

@blacksteed232 that should be fixed by this other PR #221

@akphi
Copy link

akphi commented Oct 16, 2019

Ah gotcha! Thank you! Are you happen to be a maintainer or know when could these changes go out in a release @xaviergonz?

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

Successfully merging this pull request may close these issues.

None yet

5 participants