cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
47.01k stars 3.18k forks source link

Improve typescript types of cy.wrap #18182

Open WIStudent opened 3 years ago

WIStudent commented 3 years ago

What would you like?

When using cy.wrap with a Promise the resulting type currently does not get inferred automatically. Instead generics need to be passed explicitly.

const getNumber = async (): Promise<number> => 42;

cy.wrap(getNumber()).then(n => {
  // n: unknown
});

cy.wrap<Promise<number>, number>(getNumber()).then(n => {
  // n: number
});

I think this is a bit inconvenient, typescript should be able to infer the type from the promise without needing to pass the generics. Take a look at the current type definitions of cy.wrap:

interface Chainable<Subject = any> {
  wrap<E extends Node = HTMLElement>(element: E | JQuery<E>, options?: Partial<Loggable & Timeoutable>): Chainable<JQuery<E>>
  wrap<F extends Promise<S>, S>(promise: F, options?: Partial<Loggable & Timeoutable>): Chainable<S>
  wrap<S>(object: S, options?: Partial<Loggable & Timeoutable>): Chainable<S>
}

By combining the second and third overload typescript is able to infer the type automatically:

interface Chainable<Subject = any> {
  wrap<E extends Node = HTMLElement>(element: E | JQuery<E>, options?: Partial<Loggable & Timeoutable>): Chainable<JQuery<E>>
  wrap<S>(object: S | Promise<S>, options?: Partial<Loggable & Timeoutable>): Chainable<S>
}

This introduces a breaking change for anyone who is currently passing the Promise generics explicitly (they would need to remove those generics), but in return it simplifies the use of cy.wrap with Promises in typescript greatly.

Why is this needed?

No response

Other

No response

lucaciraolo commented 1 year ago

In case anyone else wants to get automatic typescript inferring for wrap, you can just add @WIStudent's improved cy.wrap definition to your cypress/support/index.ts file

// cypress/support/index.ts
declare global {
  namespace Cypress {
    interface Chainable {
      wrap<E extends Node = HTMLElement>(
        element: E | JQuery<E>,
        options?: Partial<Loggable & Timeoutable>,
      ): Chainable<JQuery<E>>;
      wrap<S>(
        object: S | Promise<S>,
        options?: Partial<Loggable & Timeoutable>,
      ): Chainable<S>;
    }
  }
}
cypress-app-bot commented 1 year ago

This issue has not had any activity in 180 days. Cypress evolves quickly and the reported behavior should be tested on the latest version of Cypress to verify the behavior is still occurring. It will be closed in 14 days if no updates are provided.

mikavilpas commented 1 year ago

I created a simple typecheck test case to prove the workaround works.

// cypress/support/commands.ts

/// <reference types="cypress" />

declare global {
  namespace Cypress {
    // https://github.com/cypress-io/cypress/issues/18182#issuecomment-1486921299
    interface Chainable {
      wrap<E extends Node = HTMLElement>(
        element: E | JQuery<E>,
        options?: Partial<Loggable & Timeoutable>,
      ): Chainable<JQuery<E>>
      wrap<S>(
        object: S | Promise<S>,
        options?: Partial<Loggable & Timeoutable>,
      ): Chainable<S>
    }
  }
}

// Test function: if this compiles, the workaround works.
// Comment the above workaround out to see it fail, remove the comments to see it pass again
const testWrap = (): void => {
  const getNumber = async (): Promise<number> => 42
  cy.wrap(getNumber()).then((n) => {
    // n: number
    console.log(n + n)
  })
}
cypress-app-bot commented 7 months ago

This issue has not had any activity in 180 days. Cypress evolves quickly and the reported behavior should be tested on the latest version of Cypress to verify the behavior is still occurring. It will be closed in 14 days if no updates are provided.

cypress-app-bot commented 6 months ago

This issue has been closed due to inactivity.

alperen-bircak commented 2 months ago

This issue does not deserve to be closed, essential for DX