Skip to content

feat: require-data-selector uses consistent rules with assignments#302

Merged
mschile merged 4 commits intocypress-io:masterfrom
MgmClientGuy0:require-data-selectors-consistency
Mar 10, 2026
Merged

feat: require-data-selector uses consistent rules with assignments#302
mschile merged 4 commits intocypress-io:masterfrom
MgmClientGuy0:require-data-selectors-consistency

Conversation

@MgmClientGuy0
Copy link
Copy Markdown
Contributor

#272 improved the checks for the require-data-selectors rule to include variable assignments. This works great, thanks for that, but it does not have the exact same semantic as if the selector was defined inline. The following is valid if defined inline:

cy.get(`[data-cy=${ELEMENT_ID}]`)

and not valid if defined in a variable:

const selector = `[data-cy=${ELEMENT_ID}]`
cy.get(selector)

This PR now reuses the exact same checks for variable declarations as the inline version. As a bonus, this now supports variable reassignments:

const selector = "[data-cy=assessment-submit]"
const anotherOne = selector;
cy.get(anotherOne)

@cypress-app-bot
Copy link
Copy Markdown

@MikeMcC399
Copy link
Copy Markdown
Collaborator

@duranbe

You might like to take a look at this PR, since you were the last contributor to make changes to the cypress/require-data-selectors.

MikeMcC399

This comment was marked as resolved.

@duranbe
Copy link
Copy Markdown
Contributor

duranbe commented Feb 26, 2026

Ah great catch, yes indeed in that case where the definition was not inline the rule was not working properly. Great addition for the re-assignment, does it works at more than one level ?

Also I agree with @MikeMcC399 - documentation should be updated to reflect this, and existing tests "should not" be updated

@MgmClientGuy0
Copy link
Copy Markdown
Contributor Author

MgmClientGuy0 commented Feb 26, 2026

Thanks for the review. I did not delete any tests, I just removed the unnecessary escaping in one of them, as pointed out by eslint. You are allowed to have backticks in strings. The tested JS is exactly the same.

'\`[data-cy=${1}]\`' === '`[data-cy=${1}]`' // true

I will add more examples to the documentation and change my commit message to mark it as a feature.

@duranbe You can chain as many assignments as you want.

@MgmClientGuy0 MgmClientGuy0 force-pushed the require-data-selectors-consistency branch from 1640d17 to 35c6f6e Compare February 26, 2026 11:47
@MgmClientGuy0 MgmClientGuy0 changed the title fix: require-data-selector uses consistent rules with assignments feat: require-data-selector uses consistent rules with assignments Feb 26, 2026
@cacieprins cacieprins self-requested a review February 26, 2026 15:12
@MikeMcC399
Copy link
Copy Markdown
Collaborator

@MgmClientGuy0

I did not delete any tests, I just removed the unnecessary escaping in one of them, as pointed out by eslint. You are allowed to have backticks in strings. The tested JS is exactly the same.

'\`[data-cy=${1}]\`' === '`[data-cy=${1}]`' // true

The "useless escapes" have been in there for 6 years, and I assumed without checking that they had some purpose. That was the reason that I requested to preserve that line in the test.

{ code: 'cy.get(\`[data-cy=${1}]\`)' }, // eslint-disable-line no-useless-escape

After running the master branch tests in vitest in VS Code I saw that the useless escapes are not passed to vitest, and so they are in fact useless and can be removed, as you have done.

image

Comment thread docs/rules/require-data-selectors.md Outdated
Comment thread docs/rules/require-data-selectors.md Outdated
Comment thread docs/rules/require-data-selectors.md
MikeMcC399

This comment was marked as resolved.

Co-authored-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
@MgmClientGuy0 MgmClientGuy0 force-pushed the require-data-selectors-consistency branch from 4133671 to db3c15c Compare March 2, 2026 07:38
@MgmClientGuy0
Copy link
Copy Markdown
Contributor Author

MgmClientGuy0 commented Mar 2, 2026

Applied the suggestions (one selector was supposed to be a class selector). Nice catch with the two code blocks, that was visible in the rendered documentation.

Copy link
Copy Markdown
Collaborator

@MikeMcC399 MikeMcC399 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the corrections! That looks good to me now.

@mschile mschile self-assigned this Mar 10, 2026
@mschile
Copy link
Copy Markdown
Contributor

mschile commented Mar 10, 2026

bugbot run

@cursor
Copy link
Copy Markdown

cursor bot commented Mar 10, 2026

Bugbot couldn't run

Bugbot is not enabled for your user on this team.

Ask your team administrator to increase your team's hard limit for Bugbot seats or add you to the allowlist in the Cursor dashboard.

@mschile mschile merged commit de98a5d into cypress-io:master Mar 10, 2026
13 checks passed
@mschile
Copy link
Copy Markdown
Contributor

mschile commented Mar 10, 2026

Thanks for the contribution @MgmClientGuy0!

@cypress-app-bot
Copy link
Copy Markdown

🎉 This PR is included in version 6.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants