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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule to enforce consistent data-testid format #55

Closed
tknickman opened this issue Dec 18, 2019 · 7 comments 路 Fixed by #56
Closed

Rule to enforce consistent data-testid format #55

tknickman opened this issue Dec 18, 2019 · 7 comments 路 Fixed by #56
Labels
new rule New rule to be included in the plugin released

Comments

@tknickman
Copy link
Contributor

Hey, great plugin!

I recently wrote a custom rule that we are using internally at my company to enforce consistent values for data-testid. (I found this plugin because I wanted to use the same name 馃槀). We have a giant monorepo and teams were using everything from camelCase to PascalCase to kebab-case to snake_case to who-knows-what-case when writing test Id's.

This rule allows specifying a regex (that can include an inferred component name) that is used to validate all data-testid values. It has proved to be useful in larger codebases because it

  1. Avoids the dreaded naming problem
  2. Allows automation and developers to move faster by inferring test Id's without having to look them up every time.

Here is what the rule config looks like

'@custom/my-secret-plugin-name/data-testid': [
  2,
  {
    testIdPattern: '^{componentName}(__([A-Z]+[a-z]*?)+)*$',
    excludePaths: ['__tests__']
  }
],

If you think this is generally useful I would be happy to submit a PR to add this rule.
Thanks!

@Belco90 Belco90 added the new rule New rule to be included in the plugin label Dec 18, 2019
@Belco90
Copy link
Member

Belco90 commented Dec 18, 2019

Hey Thomas!

Thanks for your message! This rule seems quite interesting and it would be awesome if you could create a PR to include it in the plugin (I want to use it now at my code base actually).

I don't fully get how few things work for this rule (like what's the exact behavior of that componentName), but I guess I can find them out when you implement the rule (including tests and proper doc). Also, would be a general rule or specific React one?

Let me know if you need help with anything related to the PR. Thanks for you contribution!

@tknickman
Copy link
Contributor Author

tknickman commented Dec 18, 2019

Sounds good! I'll get a PR together.

I think this can be general rule. componentName is react specific, but it's optional in the regex. If it's present, I dynamically build the regex with the inferred component name of the current file being linted.

It's tricky to determine the actual component name via AST, so I use the file name as a stand-in (unless it's index, then I use the parent directory).

Always open to feedback / improvements so happy to collaborate on it!

@Belco90
Copy link
Member

Belco90 commented Jan 13, 2020

馃帀 This issue has been resolved in version 1.4.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

@wingy3181
Copy link

Question: looking at the code and the tests...is it true that this only works for JSX?
I am trying to use this rule on an angular app but can't seem to get it working...im assuming the rule doesn't support html (from what i can see from browsing the code)

@tknickman
Copy link
Contributor Author

@wingy3181 that is correct - see here for detail on the rule.

I'm sure contributions would be welcome to make it work for angular/html as well, I sadly don't have the time at the moment to make those additions.

@wingy3181
Copy link

@tknickman thanks for the quickly reply...yep that's the same line i saw...if i have some time i will see if i can make those additions.....

@Belco90
Copy link
Member

Belco90 commented Aug 10, 2021

Hey @wingy3181, thanks for bringing this up!

I didn't realize this was an exclusive React rule, to be honest, we should have made that clear in the rule description at least.

Would you mind reporting this in a new issue so we can track and handle it properly? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new rule New rule to be included in the plugin released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants