[RFC] feat(testing-library-dom): add esm mirror of testing-library-dom

[Westbrook]

Explainer

I really like the ideas behind the testing philosophy that @testing-library/dom espouses. Beyond its philosophy it also sets the table for a more uniform testing approach by focusing on the content of an element and how that content effects the accessibility of the element. However, the library is distributed with a number of CJS dependencies that prevent it from working with Karma, not to mention the issues of introspecting Shadow DOM. I'm not fully decided myself as to whether the cost outlined in this PR are worth the benefits of being able to use Testing Library generally and hoped to share my thoughts and findings to empower gather ideas in this are from the rest of the team.

Description

Add an ES Module mirror of @testing-library/dom to the project with a little bit of Shadow DOM hacking so that we can leverage the a11y/content centric selectors of the library in our code. I was hoping to be able to avoid majorly monkey patching the actual code here, but one is needed:

querySelectorAll: 'querySelectorAllWithShadowDOM',

Even with this one change, it feels like we could easily keep up with changes upstream. That being so, I think we'd be in an OK place to take on the publishing responsibilities outline here

The switch from querySelectorAll to querySelectorAllWithShadowDOM is also trouble. I wanted to adjust HTMLElement.prototype.querySelectorAll directly, but it alters element functionality, too. This allows the query to act more like the render does and avoid not slotted light DOM content while being able to pierce shadow roots. Being it's for tests, any performance issues seem like they'd be less devastating.

Usage

Generally, the issue that @testing-library/dom attempts to correct for in the testing lifecycle is the tendency to rely too heavily on implementation details to ensure coverage.

One place where this arises in the use of snapshot testing, while the DOM is one form of output for an element it is subsequently reprocessed at its intersection with CSS in a way that can find it changing more often than a human can process the diffs in a snapshot test. Here I've shown how ensuring that actual content (and not just a specific DOM element) is available can replace this sort of test. Notice the change in the sp-actionbar test to no longer rely on snapshot testing and instead query elements by content rather than by structure or selector.

Even when not relying on snapshot testing, it is easy to fall into the trap of relying on DOM implementation testing. See the changes to sp-sidenav that updates a recently created test to rely on presence of content as opposed to the details of a template. The value of this may not be immediately clear for this element in particular, but imaging a world where the expanded content needed to be displayed in an overlay, in that way the queryByText(el, 'Section 1') calls would only need the el to be updated to document in order to use the same over structure of test, greatly reducing refactoring costs. This would be very clearly felt via #157 which will require just that sort of change.

Motivation and Context

While testing a unit of shadow DOM is pretty straight forward the steps required to move towards integrations or e2e testing can be less clear, this will hopefully surface some possibilities there.

How Has This Been Tested?

It's test?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I have signed the Adobe Open Source CLA.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[benjamind]

I'm definitely in favor of our tests moving away from snapshot and towards testing semantics and exposed interface surface (content) rather than internal shadowdom details.

One thought came up when looking at the hacks being made to the @testing-library/dom. Rather than monkeypatching the use of querySelectorAll, could we not create a Proxy over the top level element that contains the elements under test, that would do the same work, but only modifying the top level elements querySelectorAll behavior, and would mean we did not modify the behavior of the elements under test?

[benjamind]

Also, looks like jsdom is going to very soon support custom elements: jsdom/jsdom#2548

Would this eliminate the need for some of this hackery?

[Westbrook]

I'll take a look at what's going on with JSDom, however my understanding is that the project is targeting being spec compliant, which would leave it in the same issue of needing to patch the query code to see through the shadow DOM.

As for the Proxy approach, would you be able to share a little more of how you think that would work. My first thought is that the approach pushes the techniques possible into the corner of unit testing only, particularly the point about it "contain[ing] the elements under test". This seems like it would both lower the flexibility of test, while raising the manual requirement to make a test work, which wouldn't be optimal. However, I don't know a lot about Proxies, so I'm sure there's much that I just can't see there.

[benjamind]

Diving into the actual implementation of getByLabelText and similar it appears that a proxy solution wouldn't work. I had hoped that they merely do top level queries, without re-querying back down the stack, but it appears getByLabelText will first query the container element for labels, and then re-queries from these labels again to find textareas and other items within them.

My idea was to just have a simple helper which you could apply to a container element to create a Proxy over the HTMLElement instance of the container, and intercept querySelectorAll on that instance only to do the custom ShadowDOM piercing. I suppose it could self re-insert the proxy down the tree as it retrieves elements, but it'll start getting messy fast.

Would definitely be better to just get this into the core library I think.

[Westbrook]

At least a little bit of interest seems to have peeked up on that end: testing-library/dom-testing-library#413 We'll see what they think of the approach so far 🤞

[Westbrook]

This gets skinnier and skinnier! Now, knowing testing-library/dom-testing-library#413 (comment) I'd almost say that instead of processing the dependency that we replace it with a warning that says you need to be using this library in an environment that supports MutationObserver to make the code to manage here even smaller.

[github-actions]

Branch Preview URLs

• Documentation Site
• Storybook

When a visual regression test fails (or has previously failed while working on this branch), its results will be avaialable at the associated URL below:

• Lightest | Medium | LTR
• Lightest | Medium | RTL
• Lightest | Large | LTR
• Lightest | Large | RTL
• Light | Medium | LTR
• Light | Medium | RTL
• Light | Medium | LTR
• Light | Medium | RTL
• Darkest | Medium | LTR
• Darkest | Medium | RTL
• Darkest | Large | LTR
• Darkest | Large | RTL
• Dark | Medium | LTR
• Dark | Medium | RTL
• Dark | Large | LTR
• Dark | Large | RTL

[github-actions]

Tachometer results for changed packages

action-bar permalink

Version	Bytes	Avg Time	vs remote	vs branch
remote	312 kB	18.96ms - 19.36ms	-	unsure 🔍 -3% - +1% -0.53ms - +0.13ms
branch	310 kB	19.10ms - 19.63ms	unsure 🔍 -1% - +3% -0.13ms - +0.53ms	-

sidenav permalink

Version	Bytes	Avg Time	vs remote	vs branch
remote	371 kB	440.86ms - 447.49ms	-	unsure 🔍 -1% - +1% -5.17ms - +4.48ms
branch	370 kB	441.02ms - 448.03ms	unsure 🔍 -1% - +1% -4.48ms - +5.17ms	-