Remove Lint Exceptions: Remove Simple-To-Fix React Lint Exceptions

[charleyf]

No description provided.

[aduth]

I don't love the idea of letting React claim stake to every function starting with the word "use", and while I acknowledge the likelihood of some confusion, the intent with these test helpers is to act as similar sorts of lifecycle helpers. I'd wonder if we could find a different convention which at least communicates a similar expectation of these utilities as operating on the test lifecycle, or if we could find a way to exempt the React rule from certain files (e.g. non-JSX).

As it stands, this creates inconsistency with a handful of other test helpers we have: https://github.com/18F/identity-idp/tree/main/app/javascript/packages/test-helpers

[charleyf]

Oh, interesting. I reverted the renaming and went with aliasing the functions like this
import { createDOM, useCleanDOM as utilizeCleanDom } from './support/dom';
5488890

As far as I can see this react lint rule is only causing lint errors in the one file (spec_helper.js). Which I think probably has to do with it's location in the directory structure.

I think this is a reasonable approach as long as it continues to just be this one file. If I discover that I'll need to add a bunch of these aliases, this is a much less attractive approach and exempting the files and/or renaming test helpers seems like a good idea.

I'm not sure about excluding .js and .ts files, I suspect that React lints those files for good reason in some situations (though I haven't thought this all the way through.

[aduth]

I'm not sure about excluding .js and .ts files, I suspect that React lints those files for good reason in some situations (though I haven't thought this all the way through.

Hm, I think this may have more to do with how we configure the plugin with ESLint, or at least with how the default behavior will enforce all rules for all files. I personally think it could make sense to limit where we apply those rules, such as using overrides to selectively opt-in specific files to the additional plugin rules, or opt-out files that we don't want to be subjected to these rules.

I can see how it might make sense for certain .ts and .js files to have React rules enforced, particularly custom hooks that don't include any JSX (example).

I think I'd be in favor of either:

1. Targeting the plugin rules to only apply to the packages where we expect JSX to exist (document-capture, components, etc).
2. Disabling the rules for these files and continuing to use useCleanDOM as originally named, either via overrides or /* eslint-disable ... */ at the top of the file.

[charleyf]

I've been trying various things and I've settled on an inline /* eslint-disable ... */ for the moment.

Reasoning:

• I think that the reason I'm not seeing lint errors about use... being React only for all the files in the test-helpers directory is that those files all have .spec. in the name. So anothe
• Targeting the plugin makes sense, but I think I would end up changing a lot more than one line of code to do this
  • The onlyyarn tsc and yarn lint are from the spec_helpers file right now.
• I'm not sure that targeting the plugin makes this much more readable than what I'm changing since I'd be turning it off/on in a few different places.