`Cypress.Commands.overwrite` typescript problem appearing in 9.1

[lgenzelis]

Current behavior

Cypress 9 added type definitions to Cypress.Commands.overwrite, and I can't seem to make sense of them.

This is what I have, in cypress/support/commands.ts:

if (Cypress.env('demoMode') === 'on') {
  // adapted from https://github.com/cypress-io/cypress/issues/249#issuecomment-670028947
  for (const commandName of ['visit', 'click', 'trigger', 'type', 'clear', 'reload'] as const) {
    // we add 2s delays for a few commands so that stakeholders can see what's going on
    const commandWithDelay = (command: (...args: unknown[]) => unknown, ...args: unknown[]) =>
      new Promise((resolve) => {
        setTimeout(() => resolve(command(...args)), 2000);
      });

    Cypress.Commands.overwrite(commandName, commandWithDelay);
  }
}

This was working fine until the new Cypress version.

Let's tackle something simpler, taken out straight from cypress docs:

Cypress.Commands.overwrite('type', (originalFn, element, text, options) => {
  if (options && options.sensitive) {
    // turn off original log
    options.log = false
    // create our own log with masked message
    Cypress.log({
      $el: element,
      name: 'type',
      message: '*'.repeat(text.length),
    })
  }

  return originalFn(element, text, options)
})

I can't get the types right, even for that seemingly simple example 😞

Desired behavior

No response

Test code to reproduce

Cypress.Commands.overwrite('type', (originalFn, element, text, options) => {
  if (options && options.sensitive) {
    // turn off original log
    options.log = false
    // create our own log with masked message
    Cypress.log({
      $el: element,
      name: 'type',
      message: '*'.repeat(text.length),
    })
  }

  return originalFn(element, text, options)
})

Cypress Version

9.1

Other

No response

[emilyrohrbough]

@lgenzelis Have you taken a look at #18915 or #18940 at all? This appears to be a duplicate issue of these.

[lgenzelis]

hey @emilyrohrbough , neither of those issues even mention Cypress.Commands.overwrite, so I don't see why it'd be a duplicate 😕 The problems with Cypress.Commands.add mentioned in those issues may have the same root cause as the problems with Cypress.Commands.overwrite, but I'm honestly not familiar enough with Cypress to tell for sure.

[emilyrohrbough]

Hey @lgenzelis -- sorry for the jump that this was a duplicate. The issues I linked both have PRs open to add additional types to Cypress.Commands to resolve type issues being seen for Cypress.Commands.add & Cypress.Commands.overrite.

[xumepadismal]

Hey @lgenzelis ! My PR #19003 should fix the issue from "Test code to reproduce" example.

Regarding the overwriting commands in loop

I'm not sure whether it is even possible currently in TS to provide typings that will support this case. I think it's ok to use some workarounds in your code. For example, this will work when/if my PR got merged:

if (Cypress.env('demoMode') === 'on') {
  // adapted from https://github.com/cypress-io/cypress/issues/249#issuecomment-670028947
  for (const commandName of ['visit', 'click', 'trigger', 'type', 'clear', 'reload'] as const) {
    // we add 2s delays for a few commands so that stakeholders can see what's going on
    const commandWithDelay = ((command: (...args: unknown[]) => unknown, ...args: unknown[]) =>
        new Promise((resolve) => {
          setTimeout(() => resolve(command(...args)), 2000);
        })) as any as Cypress.CommandFnWithOriginalFn<typeof commandName>;

    Cypress.Commands.overwrite(commandName, commandWithDelay);
  }
}

[dclowd9901]

Yep, it looks like the type for overwrite blindly associates the type for the overwritten function with its definition in Chainable. The solution here is that certain chainables (like visit, in my case) need to have a separate definition that is referenced outside of Chainable. It's no good to add one to Chainable since it conflates the types too much.

[lgenzelis]

Thanks @xumepadismal ! That would be a big improvement over the current state. So, what you're saying is that you don't think it's possible to add types to the case where we return a promise in the overwritten command, right?

Let's stick to the easier case from the docs. So, this would work (just copy-pasting the same example again):

Cypress.Commands.overwrite('type', (originalFn, element, text, options) => {
  if (options && options.sensitive) {
    // turn off original log
    options.log = false
    // create our own log with masked message
    Cypress.log({
      $el: element,
      name: 'type',
      message: '*'.repeat(text.length),
    })
  }

  return originalFn(element, text, options)
})

but this wouldn't:

Cypress.Commands.overwrite('type', async (originalFn, element, text, options) => {
  if (options && options.sensitive) {
    // turn off original log
    options.log = false
    // create our own log with masked message
    Cypress.log({
      $el: element,
      name: 'type',
      message: '*'.repeat(text.length),
    })
  }

  return originalFn(element, text, options)
})

(notice the async in the 1st line)

I don't know about the intricacies of cypress typing, but if Cypress.Commands.overwrite return type is Blah, can't you just change it to Blah | Promise<Blah> ?

[xumepadismal]

I'm not sure whether it is even possible currently in TS to provide typings that will support this case.

@lgenzelis I meant the for-of loop problem, not the Promises itself. TS not very good with narrowing types for the iterable.

Regarding the async functions or returning a Promise I had some trouble with typing those cases, too. The approach like Blah | Promise works bad here because you'll end up TS requiring you to return Chainable | Promise<Chainable> which is kind of absurd. I'm not saying this is impossible to fix but will likely require to adjust typings in places outside the custom commands, too. It feels like out of scope of my PR now.

AFAIK docs officially don't explicitly support this so I think we should go with a separate follow-up PR to address this and update the docs.

For now you can use return cy.wrap(Promise.resolve(originalFn(element, text, options))) which doesn't contradict the docs.

[lgenzelis]

Hey @xumepadismal , I'm looking for a way to try your PR to check it out, but I'm being unsuccessful 😞 Mind giving me a hand when you've got a minute please?

Things I've tried:

• in package.json in my repo, I replaced "cypress": "9.1.0" by "cypress": "https://github.com/xumepadismal/cypress.git#commands-types",. I update node to v16.13 (otherwise it would complain about it) and yarn install. That took a long time and ended with

error /Users/lucas.genzelis/my-repo/node_modules/cypress: Command failed.
Exit code: 127
Command: patch-package && ./scripts/run-if-not-ci.sh yarn-deduplicate --strategy=highest && ./scripts/run-if-not-ci.sh yarn build
Arguments: 
Directory: /Users/lucas.genzelis/my-repo/node_modules/cypress

• as an alternative, on my project, I replaced the content of node_modules/cypress/types/cypress.d.ts with the content of cli/types/cypress.d.ts on your branch (I don't know which other files I should have copied). After that, I re-tried the issue from "Test code to reproduce" example, and checks fail anyway

Cypress.Commands.overwrite('type', (originalFn, element, text, options) => {
  if (options?.sensitive) {
    // turn off original log
    options.log = false;
    // create our own log with masked message
    Cypress.log({
      $el: element,
      name: 'type',
      message: '*'.repeat(text.length),
    });
  }

  return originalFn(element, text, options);
});

checking that raises the error

error TS2769: No overload matches this call.
  Overload 1 of 2, '(name: "type", fn: CommandFnWithOriginalFn<"type">): void', gave the following error.
    Argument of type '(this: Context, originalFn: CommandOriginalFn<"type">, element: string, text: Partial<TypeOptions> | undefined, options: Parameters<ChainableMethods<any>[T]>[2]) => Chainable<...>' is not assignable to parameter of type 'CommandFnWithOriginalFn<"type">'.
  Overload 2 of 2, '(name: "type", fn: CommandFnWithOriginalFnAndSubject<"type", unknown>): void', gave the following error.
    Argument of type '(this: Context, originalFn: CommandOriginalFn<"type">, element: string, text: Partial<TypeOptions> | undefined, options: Parameters<ChainableMethods<any>[T]>[2]) => Chainable<...>' is not assignable to parameter of type 'CommandFnWithOriginalFnAndSubject<"type", unknown>'.
      Types of parameters 'originalFn' and 'originalFn' are incompatible.
        Type 'CommandOriginalFnWithSubject<"type", unknown>' is not assignable to type 'CommandOriginalFn<"type">'.
          Types of parameters 'text' and 'options' are incompatible.
            Type 'Partial<TypeOptions> | undefined' is not assignable to type 'string'.
              Type 'undefined' is not assignable to type 'string'.

Maybe I should also copy some other files from your branch?

[cypress-bot]

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.

[xumepadismal]

Hey @lgenzelis Please refer to this new paragraph in docs

It is from my PR cypress-io/cypress-documentation#4263 to Cypress Docs repo

[lgenzelis]

Awesome! I can't wait to see it released! 😄 Thanks @xumepadismal

[cypress-bot]

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.