ember-test-waiters icon indicating copy to clipboard operation
ember-test-waiters copied to clipboard

waitFor:

Open NullVoxPopuli opened this issue 4 years ago • 9 comments

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;

NullVoxPopuli avatar Mar 18 '21 16:03 NullVoxPopuli

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...

rwjblue avatar Mar 18 '21 17:03 rwjblue

:thinking: taskFor returns a Task: https://github.com/chancancode/ember-concurrency-ts/blob/5f76b32028bfb298b4a41e7853532949b3d42430/tests/integration/ember-concurrency-ts-async-test.ts#L32 Do we need to include all that?

NullVoxPopuli avatar Mar 18 '21 22:03 NullVoxPopuli

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 avatar Apr 14 '21 02:04 scalvert

Should it be PropertyDescriptor | unknown?

scalvert avatar Apr 14 '21 15:04 scalvert

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 avatar Sep 01 '22 07:09 Techn1x

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'))

Techn1x avatar Sep 01 '22 08:09 Techn1x

@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);
    });

i-avm avatar Nov 18 '22 08:11 i-avm

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 () => {...})

ternavsky avatar Feb 07 '23 10:02 ternavsky

For anyone watching this issue, this PR when merged should resolve it. Is anyone here able to merge it? https://github.com/emberjs/ember-test-waiters/pull/459

Will need e-c 3.1.0 and the syntax is

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

Techn1x avatar Aug 03 '23 23:08 Techn1x