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

Typings for Command.add are missing types for the subject #18879

Closed
SimeonC opened this issue Nov 11, 2021 · 11 comments · Fixed by #19003
Closed

Typings for Command.add are missing types for the subject #18879

SimeonC opened this issue Nov 11, 2021 · 11 comments · Fixed by #19003
Labels
v9.0.0 🐛 Issue present since 9.0

Comments

@SimeonC
Copy link

SimeonC commented Nov 11, 2021

Current behavior

A simple custom command that relies on prevSubject is not correctly typed.

Desired behavior

The "previous subject" argument should be correctly added to the command type and error if not passed.

Test code to reproduce

declare global {
  namespace Cypress {
    interface Chainable {
      confirmClick(): void;
    }
  }
}

Cypress.Commands.add('confirmClick', { prevSubject: true }, (confirmButton) => {
  confirmButton.click();
  cy.wrap(confirmButton)
    .should('have.attr', 'data-state', 'confirming')
    .click({ force: true });
});

Cypress Version

9.0.0

Other

No response

@omersomuncu
Copy link

For me "confirmClick" is not. Neither add nor overwrite is working at the moment.

@SimeonC
Copy link
Author

SimeonC commented Nov 11, 2021

I have done some attempts at fixing this for add (though I think the fix only works in TS 4.4) but overwrite is significantly more difficult as we don't have Options to switch on.

@jennifer-shehane jennifer-shehane added the v9.0.0 🐛 Issue present since 9.0 label Nov 11, 2021
@SimeonC
Copy link
Author

SimeonC commented Nov 12, 2021

My current workaround for this locally is to use a wrapper function as below and change

- Cypress.Commands.add('confirmClick', { prevSubject: true }, (confirmButton) => {
+ addCypressCommand('confirmClick', { prevSubject: true }, (confirmButton) => {
type AddSubjectArgument<
  TPrevSubject extends Cypress.CommandOptions['prevSubject'],
  TFunction extends (...args: any) => any
> = (
  ...args: [
    TPrevSubject extends true
      ? JQuery | HTMLElement
      : undefined | JQuery | HTMLElement,
    ...Parameters<TFunction>
  ]
) => ReturnType<TFunction>;

function addCypressCommand<
  T extends keyof Cypress.Chainable,
  TOptions extends Cypress.CommandOptions
>(
  name: T,
  options: TOptions,
  fn: Cypress.Chainable[T] extends (...args: any) => any
    ? TOptions['prevSubject'] extends false
      ? Cypress.Chainable[T]
      : AddSubjectArgument<TOptions['prevSubject'], Cypress.Chainable[T]>
    : never
): void {
  Cypress.Commands.add<T>(name, options, fn as Cypress.Chainable[T]);
}

@kubijo
Copy link

kubijo commented Nov 12, 2021

My current workaround for this locally is to use a wrapper function as below and change

- Cypress.Commands.add('confirmClick', { prevSubject: true }, (confirmButton) => {
+ addCypressCommand('confirmClick', { prevSubject: true }, (confirmButton) => {

EDIT: I was proposing a solution, but I spoke too soon and later found out that it actually doesn't work, so I'm removing it to prevent confusion

@SimeonC
Copy link
Author

SimeonC commented Nov 12, 2021

My workaround does handle all values of prevSubject, just in 2 places rather than one.

  fn: Cypress.Chainable[T] extends (...args: any) => any
    ? TOptions['prevSubject'] extends false
      ? Cypress.Chainable[T]
      : AddSubjectArgument<TOptions['prevSubject'], Cypress.Chainable[T]>
    : never

At this place we check if prevSubject is false, which means that the subject should not be added - hence returning Cypress.Chainable[T] otherwise we pass to the AddSubjectArgument type.

Key is this part of the type at this point, our type of TPrevSubject is true | string | string[] (OK, not quite string cause it's 'internal', 'window' etc). So the ternary correctly handles between being true and the value cannot be undefined, or the other values where it can be undefined.

...args: [
    TPrevSubject extends true
      ? JQuery | HTMLElement
      : undefined | JQuery | HTMLElement,
    ...Parameters<TFunction>

This is the solution I used in the attached PR which passes all the TS 4 tests, the only problem I have is that this only seems to work in TS 4 so the TS 3 tests fail.

@maciaszczykm
Copy link

We have the same problem, typings are not available for child commands since #17496 was introduced.

@Threnos
Copy link

Threnos commented Nov 17, 2021

Bump

@cypress-bot cypress-bot bot added stage: waiting and removed stage: needs review The PR code is done & tested, needs review labels Nov 18, 2021
@FrancisBourgault
Copy link

Same issue here.

All my commands already have typescript declaration, but now it breaks because the prevSubject parameter isn't present in the declaration (and it shouldn't be, since it's not directly provided by the user)

@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: waiting labels Nov 29, 2021
@SimeonC
Copy link
Author

SimeonC commented Nov 30, 2021

This will now be resolved in #19003

@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Dec 13, 2021
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 13, 2021

The code for this is done in cypress-io/cypress#19003, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 21, 2021

Released in 9.2.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v9.2.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Dec 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
v9.0.0 🐛 Issue present since 9.0
Projects
None yet
7 participants