I made a custom assertion that checks if an element is in the viewport (fails if need to scroll to it), and I want to execute it by default before clicking on anything. So I added this:

js
Cypress.Commands.overwrite("click", (originalFn, subject, options) => {
  if (options !== undefined && options.scroll) {
    originalFn(subject, options)
  } else {
    cy.wrap(subject).should("be.in-viewport").then(() => originalFn(subject, options))
  }
})

But this gives me an error on another part of the code saying

Cypress detected that you returned a promise from a command while also invoking one or more cy commands in that promise.

The command that returned the promise was:

  > cy.clear()

The cy command you invoked inside the promise was:

  > cy.wrap()

Because Cypress commands are already promise-like, you don't need to wrap them or return your own promise.

Cypress will resolve your command with whatever the final Cypress command yields.

I don't understand, what am I doing wrong? I'm not returning a promise...

I'm gonna take a closer look but I know Gleb has a helpful video about this subject so as a starting point this is worth checking out

This is my test (edited)``` -snip- ```

I am a little puzzled as I was able to overwrite click without a problem just now 🤔. Let me look at those issues for a second. What version of Cypress are you using by the way? Based on the error, it looks more like these lines are the problem, not the custom command, for what it's worth:

cy.get("[data-cy-edit-form-name=Gunjo]").within(() => {
      cy.get("input[name=name]").clear().type("Gunjont") // <- here
    })

cypress10.0.1

yeah I'm seeing the same thing, if I am overriding

click

in this way, I get the error. Which makes me think maybe

clear

is using

click

this also happens with type()

pretty sure type also uses

click

internally to ensure the element you are typing into has focus, need to double check

guess ill just add a new one instead ¯\_(ツ)_/¯

yeah that would be safer, I feel we should either warn when a user is overwriting

click

, or make an internal private

click

that other commands use. Not sure what's best. But

clear

on a form field is an alias for

.type({selectall}{backspace})

so if

click

is invoked when you

type

it's also invoked when you

clear

Yeah that's a good idea. Also I'm looking through my tests and I assumed that most of the time I would want to fail if the element is outside of the viewport but actually most of the time I do want the scrolling behavior. The problem is that for some reason this scrolling extends to elements that are scrolled out to the right of the screen but I only want it to go up or down