fix: Allow multipart jsx node names, data-qa on new line for multiline jsx open tags.

[pixelbandito]

Before this commit, node names with dots in them caused an error in the rule.

A file containing the following was not lintable, and projects that relied on successful linting to build, commit, etc. would fail.

<Media.Item onClick={() => {}} />

In this change, I'm switching to using node.name.range to get the position for inserting the attribute. That method works for both single and multi-identifier jsx elements.

Also, as a convenience, I'm detecting if a jsx element is on 2 lines, and, if so, adding a newline and indentation. The indentation is simplistic: the same column as the tag opener plus 2 spaces.

<Thing
  onClick={foo}
/>
// becomes
<Thing
  data-qa="os_1hG-zWpPAKZzA0t34o"
  onClick={foo}
/>

Finally, when running npm run lint on the repo before committing, there were some lint errors around indentation for this project and preferring template strings over concatenating strings with +, so I fixed those.

[dkordik]

reading your description and realizing this 2 is a hardcoded soft tab indent. do we have visibility into the next line's indent? might be nice if we indented based on the existing indent convention if it's easy

[pixelbandito]

I could probably do that.
The other devil's advocate point (against me) is that the attribute indentation / newlining should be handled by a separate lint rule, in which case the test-selectors plugin should just butt out.

Honestly, that's probably the cleaner approach.

[pixelbandito]

If I routinely ran prettier after linting, this wouldn't have been an issue. 🤦

[dkordik]

I ultimately like the idea of adding the fix in the "right place" (e.g. next line, indented)- I know at least in our world, auto-formats are a separate, distinct step from ESLint corrections, so if it "just worked", that'd feel pretty slick. but I think it'd potentially be worse to indent wrong than to not try to indent at all

[pixelbandito]

this might read a little nicer as a multi-line string. any reason that isn't an option?

Because indentation mattered, I thought this gave better visibility into the relative indentation levels than doing

        {
            code: `<Foo.Bar\n
  onClick={ () => handleClick() }
  readonly={ foo }
>
  foo
</Foo.Bar>`,
            errors: [onClickError],

[pixelbandito]

@dkordik I removed the multi-line special case. Consumers are expected to provide their own linting / prettier config so they're in full control of indentation and when to break lines.

[dkordik]

lgtm! fyi, we have an active PR from our master branch to the main line, so when we merge this, we should see that reflected in that downstream PR as well: davidcalhoun#8