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

Unable to overwrite click command #19086

Closed
oririner-wiz opened this issue Nov 24, 2021 · 8 comments
Closed

Unable to overwrite click command #19086

oririner-wiz opened this issue Nov 24, 2021 · 8 comments
Labels
stale no activity on this issue for a long period

Comments

@oririner-wiz
Copy link

Current behavior

I want to overwrite click with the following code:

Cypress.Commands.overwrite('click', (originalClick, subject, ...args) => {
  originalClick(subject, ...args);

  cy.window().then(window => new Cypress.Promise(resolve => window.requestAnimationFrame(() => window.requestAnimationFrame(resolve))));

  cy.wrap(subject);
});

I want to be able to "pause" after clicks till after a frame has passed because of some 3rd-party library doing some DOM side-effects during the animation frame. This way I clicked something, I wait for the 3rd-party library to do it's side-effect and I can now continue with the normal flow.

However, this creates some issues.

I'm also using cy.type & cy.check which internally use cy.click, but they don't do it "normally" as a command but rather through cy.now('click', ...). It would've been fine but cy.now (which is undocumented) wraps the command with a "promise" and my call to cy.window within the overwritten command also creates a "promise" and then we see the big red error message banners that you can't use promise in a command inside cypress.

I tried doing as suggested here but it caused cy.type to type at the start of the input and not at the end (I have tests where I append some text and they failed because of it)
Regardless, I don't think I should be overwriting check, uncheck, type, select (and possibly any other command in the future that triggers cy.now('click', ...)) just because I want to overwrite click, this feels very brittle.

Desired behavior

Be able to overwrite click just like any other command and invoke any other cy commands inside of it.

Test code to reproduce

// cypress/support/commands.js
Cypress.Commands.overwrite('click', (originalClick, subject, ...args) => {
  originalClick(subject, ...args);

  cy.window().then(window => new Cypress.Promise(resolve => window.requestAnimationFrame(() => window.requestAnimationFrame(resolve))));

  // "Returns" the original subject from the overwritten command
  cy.wrap(subject);
});
// cypress/integration/example.spec.js
it('should check a checkbox', () => {
  cy.visit('https://yari-demos.prod.mdn.mozit.cloud/en-US/docs/Web/HTML/Element/input/checkbox/_sample_.Handling_multiple_checkboxes.html');

  // This fails
  cy.get('input[type="checkbox"][value=music]').check();
});

Here's how it looks like:
CleanShot 2021-11-24 at 14 44 56@2x

Cypress Version

7.5.0

Other

I know I'm not the most recent version, but I looked at the code in develop and it's still with cy.now('click', ...), so that wouldn't work.

Anyway, thank you for all your hard work!
It's really appreciated 🙏

@akselil
Copy link

akselil commented Jan 7, 2022

The same problem here with 9.2 cypress.

@oririner-wiz
Copy link
Author

@emilyrohrbough I don't think it's related to typescript, only partially when you need to override with different parameters, but that's not my use case.

Can you please update the labels accordingly?

@oririner-wiz
Copy link
Author

@emilyrohrbough
Can someone please take a look at this?

@marktnoonan
Copy link
Contributor

@oririner-wiz I see your point here, and I like your solution to bake-in the waiting for the 3rd-party stuff to complete. In the mean time, what you have you done to achieve this waiting? Do you have a command called clickAndAwaitAnimationFrame or similar, that you use in place of click?

I'm not involved in planning work around this, but just curious about the underlying desire (plus I agree, this does not seem to need the typescript label unless I'm missing something). It might be useful to know when a person looks at a spec file, that the click is a custom command, and no longer the default Cypress behavior, so I might gravitate to a custom name anyway, but that's just me.

I do think even in the current state it would be good to at least warn that, when you overwrite click, you are overwriting something that is invoked by multiple other commands, which is probably not what is intended. The default "promise" error coming from a check or type is not particularly easy to connect to your custom click command. In fact somebody asking about this is what led me to this issue.

@oririner-wiz
Copy link
Author

Hey @marktnoonan thanks for looking into this 🙏

I indeed created a new command, it's called pick (because "luckily" it happens only on dropdown menus in my case).

I actually disagree about creating a custom command, cypress has many commands and adding to it more custom commands creates a very large api surface on the cy namespace. When one writes a test it becomes really hard to know what are the methods they can invoke. Sticking to overwriting existing commands reduces this api surface and cognitive load when writing a test. Assuming of course the new logic can be derived from the current command + application context, like in my case, clicks are usually acted on 3rd party components that usually needs to be waited with an animation frame.

We have a local docs for all of the custom commands to allow for discovering these new custom command, but still it's usually less visited vs the cypress api docs.

With that said, it would be really nice to know which commands are overwritten at a glance in the IDE, but that's more of a nice-to-have IMO. A better way to go about this would be to avoid the global cy namespace and explicitly importing each command when you need it. You wouldn't need to worry about overwriting or anything, just import the click command from your own custom commands folder instead of cypress (which in turn imports click from cypress). Anyway, that's just a thought, or more like a wish :)

I do think even in the current state it would be good to at least warn that, when you overwrite click, you are overwriting something that is invoked by multiple other commands, which is probably not what is intended.

Totally agree! though I think that with the current api, it's hard because it's only possible to know which commands are used in your command during runtime, and if your command isn't used. Maybe adding "dependencies" to each command so that you can define this dependency graph of commands that would allow you to know when you're overwriting a command that's being used by other commands.

@MoKhajavi75
Copy link

Still exist in Cy v12
I want to extend click options to add some useful functionality but I'm stuck 😕

@cypress-app-bot
Copy link
Collaborator

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 cypress-app-bot added the stale no activity on this issue for a long period label Sep 6, 2023
@cypress-app-bot
Copy link
Collaborator

This issue has been closed due to inactivity.

@cypress-app-bot cypress-app-bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale no activity on this issue for a long period
Projects
None yet
Development

No branches or pull requests

6 participants