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 error thrown when chaining should() command to scrollIntoView #5160

Open
tgdevereux opened this issue Mar 29, 2023 · 6 comments · May be fixed by #5161
Open

unsafe-to-chain-command error thrown when chaining should() command to scrollIntoView #5160

tgdevereux opened this issue Mar 29, 2023 · 6 comments · May be fixed by #5161
Labels
E2E triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team.

Comments

@tgdevereux
Copy link

tgdevereux commented Mar 29, 2023

In the latest version of eslint-plugin-cypress, version 2.13.2, cypress/unsafe-to-chain-command lint errors appear to be getting thrown for scrollIntoView commands, when 'should' test validations are chained to the command, i.e. cy.get('button#checkout').scrollIntoView().should('be.visible')

In the Cypress docs, it lists this type of verification as correct/safe usage of the scrollIntoView command, i.e. https://github.com/cypress-io/cypress-documentation/blob/main/docs/api/actions/scrollintoview.mdx#scrolling

Is this unexpected behavior in the latest eslint-plugin-cypress version?

@tgdevereux tgdevereux changed the title unsafe-to-chain-command error thrown when chaining should command to scrollIntoView unsafe-to-chain-command error thrown when chaining should() command to scrollIntoView Mar 29, 2023
@emilyrohrbough
Copy link
Member

@tgdevereux I re-read the documentation you linked (updated with a recent doc re-organization change- live here) and it mentions in two places that the .scrollIntoView() command is unsafe to chain further.

It appears that eslint-plugin-cypress@2.13.2 is correctly linting for the scrollIintoView() command.

Thank you for verifying!

@tgdevereux
Copy link
Author

tgdevereux commented Mar 31, 2023

@emilyrohrbough The Cypress docs you linked appears to suggest that chaining a ".should()" verification to a .scrollIntoView() command is correct usage, via the example they give here: https://docs.cypress.io/api/commands/scrollIntoView#Scrolling
This is the scrolling example listed:
cy.get('button#checkout').scrollIntoView().should('be.visible')

That example seems to suggest thats correct usage, does that constitute a ' command that relies on the subject after'? The scrollIntoView() command wouldn't really be useful if it couldn't be used in a test command like that

@emilyrohrbough
Copy link
Member

@tgdevereux I see, I was looking specifically at the sections that call out It is [unsafe](https://docs.cypress.io/guides/core-concepts/retry-ability#Only-queries-are-retried) to chain further commands that rely on the subject after .scrollIntoView()..

This example needs updated.

@emilyrohrbough emilyrohrbough transferred this issue from cypress-io/eslint-plugin-cypress Mar 31, 2023
@emilyrohrbough emilyrohrbough linked a pull request Mar 31, 2023 that will close this issue
@nagash77 nagash77 added triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. E2E labels Apr 20, 2023
@GWellerGMSL
Copy link

Why on earth does it return Chainable if you're not supposed to use it that way?

@MikeMcC399
Copy link
Contributor

The incorrect example has moved to https://docs.cypress.io/api/commands/scrollIntoView#Scrolling with unchanged contents:


Examples

Scrolling

cy.get('button#checkout').scrollIntoView().should('be.visible')

It's unclear why the documentation was never corrected, although https://example.cypress.io/commands/actions shows usage without contravening the unsafe-to-chain rule.

See https://github.com/cypress-io/cypress-example-kitchensink/blob/90c796eac161e3b5f578ef56cbe3406339929ff4/cypress/e2e/2-advanced-examples/actions.cy.js#L235-L265

  it('.scrollIntoView() - scroll an element into view', () => {
    // https://on.cypress.io/scrollintoview


    // normally all of these buttons are hidden,
    // because they're not within
    // the viewable area of their parent
    // (we need to scroll to see them)
    cy.get('#scroll-horizontal button')
      .should('not.be.visible')


    // scroll the button into view, as if the user had scrolled
    cy.get('#scroll-horizontal button').scrollIntoView()
    cy.get('#scroll-horizontal button')
      .should('be.visible')


    cy.get('#scroll-vertical button')
      .should('not.be.visible')


    // Cypress handles the scroll direction needed
    cy.get('#scroll-vertical button').scrollIntoView()
    cy.get('#scroll-vertical button')
      .should('be.visible')


    cy.get('#scroll-both button')
      .should('not.be.visible')


    // Cypress knows to scroll to the right and down
    cy.get('#scroll-both button').scrollIntoView()
    cy.get('#scroll-both button')
      .should('be.visible')
  })

@MikeMcC399
Copy link
Contributor

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

Successfully merging a pull request may close this issue.

5 participants