-
Notifications
You must be signed in to change notification settings - Fork 89
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
feat(rule): add 'require-data-selectors' rule #30
feat(rule): add 'require-data-selectors' rule #30
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be better named no-brittle-selectors
, since that is its purpose.
I like how it is not part of the recommended rules, since some projects do have valid use cases for them; however it could be enabled in the default if it's set to warn
error level.
lib/rules/no-dynamic-id-classes.js
Outdated
docs: { | ||
description: 'Use data-* attributes to provide context to your selectors and insulate them from CSS or JS changes https://docs.cypress.io/guides/references/best-practices.html#Selecting-Elements', | ||
category: 'Possible Errors', | ||
recommended: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this doesn't do anything, but should be false
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bkucera What about calling it something like require-data-selectors
, since that's what it basically doing?
Ready for a re-review 🙇 |
the author has addressed the comments
@bkucera could you review this PR again - the author has addressed your comments I think |
🎉 This PR is included in version 2.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This new rule should probably be added to the table in the README as well? 🙂 |
@Svish good catch, PRs welcome |
Hi -- there's no way I know of getting this to work with a helper function: e.g. export const getTestId = (id: string) => `[data-testid="${id}"]`;
getTestId('headernav') will there be functionality to support this? thanks! |
…slint-plugin-eslint-plugin-5.0.4
Goal :- Add best practices rule related to https://docs.cypress.io/guides/references/best-practices.html#Selecting-Elements
Testing:- Added few unit tests