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

'unsafe-to-chain-command' has too many false positives #5216

Open
microbae opened this issue Apr 5, 2023 · 8 comments
Open

'unsafe-to-chain-command' has too many false positives #5216

microbae opened this issue Apr 5, 2023 · 8 comments
Labels
content: rewrite triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team.

Comments

@microbae
Copy link

microbae commented Apr 5, 2023

Is it intended that the 'unsafe-to-chain-command' rule hits, if you use quite normal stuff like the ones given in the official Cypress documentation?
I think the rule should do much more than just checking if the action is included in the unsafeToChainActions array. There are too many false positives here..

Example:

See https://docs.cypress.io/api/commands/blur
cy.get('[name="comment"]').focus().type('Nice Product!').blur()

@nagash77
Copy link
Contributor

nagash77 commented Apr 5, 2023

@microbae Can you elaborate a bit more on why you think this is a false positive and what you think the correct behavior should be?

@riboher
Copy link

riboher commented Apr 19, 2023

Hi! There are still some cases in which this eslint rule is causing an error even when the code looks exactly the same as some examples in the cypress documentation.

cy.get('.map-container').scrollIntoView().should('be.visible');

This is causing an error and it's something that is widely used and showed in the cypress docs

@mjhenkes
Copy link
Member

@riboher as i understand it from here: https://docs.cypress.io/guides/core-concepts/retry-ability#Actions-should-be-at-the-end-of-chains-not-the-middle

That command should be broken up where you end chains after an action.

cy.get('.map-container').scrollIntoView();
cy.get('.map-container').should('be.visible');

I think the blur command should be broken up too.

cy.get('[name="comment"]').focus().type('Nice Product!')
cy.get('[name="comment"]').blur()

So it seems like the cypress docs need to be updated.

@mjhenkes mjhenkes assigned mschile and unassigned mjhenkes Apr 25, 2023
@mschile mschile transferred this issue from cypress-io/eslint-plugin-cypress Apr 25, 2023
@mschile
Copy link
Contributor

mschile commented Apr 25, 2023

Transferred this issue to the cypress-documentation repo so the documentation examples can be updated.

@mschile mschile added the triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. label Apr 25, 2023
@mschile mschile removed their assignment Apr 25, 2023
@nagash77 nagash77 self-assigned this May 9, 2023
@nagash77 nagash77 assigned elylucas and unassigned nagash77 May 22, 2023
@elylucas elylucas removed their assignment Jul 31, 2023
@alex-w0
Copy link

alex-w0 commented Sep 1, 2023

I experienced the same issue with cy.focused().blur() and I don't think this should be split up.

@tonai
Copy link

tonai commented Sep 11, 2023

Same for me, I have an eslint error on the following line:

cy.focused().should('have.id', 'name');

I don't think I can split this.

@dominicfraser
Copy link

For

cy.focused().should('have.id', 'name');

the docs explicitly say this is safe, but it does flag on cypress/unsafe-to-chain-command in eslint-plugin-cypress 2.14.0

https://docs.cypress.io/api/commands/focused#Syntax

cy.focused() is a query, and it is safe to chain further commands.

I've made a PR here: cypress-io/eslint-plugin-cypress#142

@MikeMcC399

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content: rewrite triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team.
Projects
None yet
Development

No branches or pull requests