Provide strong types for 'invoke' command (#4022)

[mayfieldiv]

Closes #4022 (1st attempt -> #4907)

Provides strong typing for arguments and return type of invoke command. Also ensures functionName is the key of a function on the Subject.

This PR is much crazier than the first in order to address the issue detailed in #4022 (comment), as well as other issues that came up, e.g. functions with no parameters.

I expanded the test cases to cover several more scenarios; not all of those tests pass under TypeScript 3.3, but work with 3.4+.

BTW, I totally understand if there's too much craziness in here to merge; I'm just glad this was a nice problem for playing with complex types.

Pre-merge Tasks

• Have tests been added/updated for the changes in this PR?
• Have the type definitions been updated with any user-facing API changes?

[bahmutov]

wow @mayfieldiv this is nice! Let us look at it, we had so much fun discussing the TS problem yesterday. I wonder about updating TS all the way to v3.4.0

[dmtrKovalenko]

Thank you so much for your work! It's really cool

[dmtrKovalenko]

I think we can expose all this types to separate types like AnyFunction5Args. I think it will not be a giant problem?

[mayfieldiv]

We could split into many separate types like:

type Process5ArgFunction<T, TArgs> =
  T extends { (...args: infer A1): infer R1; (...args: infer A2): infer R2; (...args: infer A3): infer R3; (...args: infer A4): infer R4; (...args: infer A5): infer R5; }
    ? ValidReturn<TArgs, A1, R1> | ValidReturn<TArgs, A2, R2> | ValidReturn<TArgs, A3, R3> | ValidReturn<TArgs, A4, R4> | ValidReturn<TArgs, A5, R5> : never
...

but I'm not sure how much that would accomplish. Is that what you mean? I think the main complication with commonization is that we need the individual inferred types.

[mayfieldiv]

wow @mayfieldiv this is nice! Let us look at it, we had so much fun discussing the TS problem yesterday. I wonder about updating TS all the way to v3.4.0

Something of note is that the results with earlier TS versions don't seem too bad.. Although I haven't looked into it too much, it seems to fall back to less restrictive return types based on the test failures.

ERROR: 338:1   expect                      TypeScript@3.3: Expected an error on this line, but found none.
ERROR: 340:1   expect                      TypeScript@3.3: Expected an error on this line, but found none.
ERROR: 352:1   expect                      TypeScript@3.3: Expected an error on this line, but found none.
ERROR: 358:1   expect                      TypeScript@3.3: Expected an error on this line, but found none.
ERROR: 359:3   expect                      TypeScript@3.3 expected type to be:
  Chainable<JQuery<HTMLInputElement>>
got:
  Chainable<HTMLInputElement | JQuery<HTMLInputElement>>
ERROR: 360:3   expect                      TypeScript@3.3 expected type to be:
  Chainable<string | number | string[] | undefined>
got:
  Chainable<{}>
ERROR: 361:3   expect                      TypeScript@3.3 expected type to be:
  Chainable<string>
got:
  Chainable<string | number | boolean | Queue<Node> | JQuery<HTMLInputElement> | Promise<JQuery<HTMLInputElement>, any, any> | undefined>
ERROR: 362:1   expect                      TypeScript@3.3: Expected an error on this line, but found none.

[jennifer-shehane]

Sorry for the delay on this. I probably messed up the merge conflicts since we've updated the invoke command since this was open.

ERROR: 336:11  no-mergeable-namespace  Mergeable namespace 'CypressInvokeTests' found. Merge its contents with the namespace on line 110.

[jennifer-shehane]

@mayfieldiv Will you be able to look at the merge conflicts + lint errors on this PR? If not, we will need to close due to inactivity.

[claassistantio]

All committers have signed the CLA.

[mayfieldiv]

@mayfieldiv Will you be able to look at the merge conflicts + lint errors on this PR? If not, we will need to close due to inactivity.

I've rebased and resolved the merge conflicts and lint errors. Everything passes, but it looks like the new version of invoke that takes an array of functions (which I'm curious of the use case) might cause a bunch of confusing error messages and potentially unwanted type errors that aren't covered by existing tests

Edit: I've added types to the invoke overload that takes an array of functions and added some tests. Everything seems to work, but I could certainly be missing some edge cases not covered by the tests.

One caveat is that in order to get the index argument to narrow the types of the parameters/return of the selected function, the user has to explicitly inform typescript that the array is actually a tuple of functions, as you can see in the tests I added. It may be necessary to add another overload that falls back to less restrictive types in the case that Subject is an array instead of a tuple of functions.

[jennifer-shehane]

I've rebased and resolved the merge conflicts and lint errors. Everything passes, but it looks like the new version of invoke that takes an array of functions (which I'm curious of the use case) might cause a bunch of confusing error messages and potentially unwanted type errors that aren't covered by existing tests

Yes, there was some debate over its usecase within our team as well 😅, but it is keeping the same signature as .its() mostly.

[bahmutov]

this is a pretty big jump. Is this normal? Should we maintain 2 versions of type definitions: one v2 and another v3?

[bahmutov]

can you namepace the input element tests more, they get lost mixed like this with wrapped object tests. Also, this needs probable needs a few complementary tests like

// checkValidity is a valid method on jQuery object, right?
cy.get('input').invoke('checkValidity') // $ExpectType Chainable<boolean>
cy.get('input').its(0) // $ExpectType Chainable<HTMLInputElement>
  // but not on the underlying DOM element
  .invoke('checkValidity') // $ExpectError

[jennifer-shehane]

@mayfieldiv Are you able to address the comments from bahmutov above related to this PR? We'll have to close the PR if you are unable to address due to inactivity.

[jennifer-shehane]

Unfortunately we have to close this PR due to inactivity. Please open a new PR addressing the original issue and any requested changes.