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

waitFor: #241

Open
NullVoxPopuli opened this issue Mar 18, 2021 · 9 comments
Open

waitFor: #241

NullVoxPopuli opened this issue Mar 18, 2021 · 9 comments

Comments

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Mar 18, 2021

The Error

error TS1236: The return type of a property decorator function must be either 'void' or 'any'.
  Unable to resolve signature of property decorator when called as an expression.

32   @waitFor
     ~~~~~~~~
error TS2769: No overload matches this call.
  Overload 1 of 3, '(fn: AsyncFunction<any[], any>, label?: string | undefined): Function', gave the following error.
    Argument of type 'MyComponentClass' is not assignable to parameter of type 'AsyncFunction<any[], any>'.
      Type 'MyComponentClass' provides no match for the signature '(...args: any[]): Promise<any>'.
  Overload 2 of 3, '(fn: CoroutineFunction<any[], any>, label?: string | undefined): CoroutineFunction<any[], any>', gave the following error.
    Argument of type 'MyComponentClass' is not assignable to parameter of type 'CoroutineFunction<any[], any>'.
      Type 'MyComponentClass' provides no match for the signature '(...args: any[]): CoroutineGenerator<any>'.

32   @waitFor
      ~~~~~~~


Found 2 errors.

This is an code that looks like:

  @task
  @waitFor
  myTask = taskFor(async () => {
   // ...

using:

  • ember-concurrency-ts
  • ember-concurrency-async
  • ember-concurrency-decorators
  • ember-concurrency @ v1.x

I believe the fix would be to make this change:

  export default function waitFor(
    target: object,
    _key: string,
    descriptor: PropertyDescriptor,
    label?: string
- ): PropertyDescriptor;
+ ): any;

This is with TypeScript 4.1.2

I've worked around this via:

import { waitFor as _waitFor } from '@ember/test-waiters';

export const waitFor = (_waitFor as unknown) as PropertyDecorator;
@rwjblue
Copy link
Member

rwjblue commented Mar 18, 2021

Hmm, I don't think we should change to returning any there. We could change it to PropertyDescriptor | SomeOtherThingIDunno. We just need to decide what that other type is...

@NullVoxPopuli
Copy link
Sponsor Contributor Author

@scalvert
Copy link
Collaborator

Task is an ember-concurrency type, so it makes more sense to me to try to use more generic types rather than those specific to particular implementations.

I can poke at this and see what I can come up with.

@scalvert
Copy link
Collaborator

Should it be PropertyDescriptor | unknown?

@Techn1x
Copy link

Techn1x commented Sep 1, 2022

Now that ember-concurrency 2.3.x includes the async "taskFor" syntax for e-c tasks, this issue's probably going to come up a bit more as folks move to the new syntax

For a task with the updated e-c syntax like this;

// book-finder/service.ts

@waitFor
searchBooks = task({ drop: true }, async (search: string) => {
  const booksResult = await this.store.query('book-library/book', { filter: { search } })
  return booksResult
})

The typescript error looks like this

Decorator function return type is 'Function & CoroutineFunction<any[], any> & PropertyDescriptor' but is expected to be 'void' or 'any'.ts(1271)
No overload matches this call.
  Overload 1 of 3, '(fn: AsyncFunction<any[], any>, label?: string | undefined): Function', gave the following error.
    Argument of type 'BookFinderService' is not assignable to parameter of type 'AsyncFunction<any[], any>'.
  Overload 2 of 3, '(fn: CoroutineFunction<any[], any>, label?: string | undefined): CoroutineFunction<any[], any>', gave the following error.
    Argument of type 'BookFinderService' is not assignable to parameter of type 'CoroutineFunction<any[], any>'.

@Techn1x
Copy link

Techn1x commented Sep 1, 2022

Actually, it turns out the types is the smaller problem now - the existing methods of using waitFor() with e-c don't seem to work with the new syntax.

For simpler tasks I've been able to get away with this format, but anything with more than one await in it gets a bit ugly

searchBooks = task({ drop: true }, async (search: string) => {
  const booksResult = await waitFor(this.store.query('book-library/book', { filter: { search } }))
  return booksResult
})

or

waitForPromise(this.searchBooks.perform('abc'))

@i-avm
Copy link

i-avm commented Nov 18, 2022

@Techn1x Could you please check the following case ?

Calling a restartableTask with waitForPromise is throwing a TaskCancelation error ( sometimes )

TaskCancelation: TaskInstance 'XYZ' was canceled because the object it lives on was destroyed or unrendered. For more information, see: http://ember-concurrency.com/docs/task-cancelation-help
                at TaskInstanceExecutor.finalizeWithCancel 
                at TaskInstanceExecutor.finalize 
                at TaskInstanceExecutor.handleResolvedReturnedValue 
                at TaskInstanceExecutor.proceedSync 
                at invoke
                at Queue.flush 
                at DeferredActionQueues.flush 
                at Backburner._end 
                at Backburner.end

Looks like, the error is thrown if a Task is restart ed when the current instance's Promise is in a pending state.

const PROMISE = waitForPromise(this.taskXYZ.perform());

    PROMISE.catch((error) => {
      console.info(error);
    });

@ternavsky
Copy link

ternavsky commented Feb 7, 2023

As long as waitFor() with ember-concurrency don't seem to work with the new syntax, I'm using this custom task modifier

import { registerModifier } from 'ember-concurrency';
import { waitFor } from '@ember/test-waiters';

function testsWaitForModifier(taskFactory, option) {
  if (!option) {
    return;
  }

  let taskDefinition = taskFactory.taskDefinition;
  let waitForDefinition = waitFor(function* (...args) {
    yield* taskDefinition.apply(this, args);
  });

  taskFactory.setTaskDefinition(waitForDefinition);
}

registerModifier('testsWaitFor', testsWaitForModifier);

export default testsWaitForModifier;

and defining task as task({ testsWaitFor: true }, async () => {...})

@Techn1x
Copy link

Techn1x commented Aug 3, 2023

For anyone watching this issue, this PR when merged should resolve it. Is anyone here able to merge it?
#459

Will need e-c 3.1.0 and the syntax is

queryStats = task(
  { restartable: true },
  waitFor(async (query: typeof this.query) => {
    // stuff
  }),
)

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

6 participants