isDisabled doesn't match the toBeDisabled() implementation in testing-library/jest-dom for custom elements

[ashleyryan]

Reproduction example

https://codesandbox.io/s/hopeful-hugle-c5xqcg?file=/src/App.test.js

Prerequisites

I can't get the tests to run in codesandbox, but in a repo, they behave as follows:

1. Notice how the button is disabled and cannot be clicked in the browser
2. Notice how in the unit test, the input is getting the text content
3. Notice how the matches expect().toBeDisabled() passes, despite the click going through.

Expected behavior

The click shouldn't happen because the button is disabled

Actual behavior

The click goes through, even though the button passes the toBeDisabled() test.

User-event version

14.0.0

Environment

"@testing-library/jest-dom": "^5.16.2",
"@testing-library/react": "^12.1.2",
"@testing-library/user-event": "^14.4.3",

Jest, using react-script v5

A sample here: vmware-clarity/starters@main...ashleyryan:starters:asryan/tl-userevent

Additional context

The reason this is happening is because in v14 of user-event, the implementation of isDisabled was changed to add a check for formAssociated for custom elements. This isn't wrong, the spec says a custom-element is disabled if it's form-associated.

However, formAssociated is intended to be used with ElementInternals, which doesn't have full browser support (Safari doesn't have it) - and is absolutely not supported in jsdom. Most custom element authors have to find workarounds for custom form elements, and in our case, we use a hidden button element in the light DOM for form submission and other form-associated behavior.

So yes, we could (theoretically) add static formAssociated = true to our elements and these tests would work, but that's not entirely correct without attachInternals() and its API being implemented. It's confusing and misleading to see that in the source code.

I helped a colleague add custom-element support to toBeDisabled() in @testing-library/jest-dom - that implementation only checks that it's a custom element, named with a -. At the time, I noticed the spec said the "form-associated" part and looked into adding the check for the static attribute, but noticed that jsdom didn't support attachInternals(), so we left it out. PR: testing-library/jest-dom#368

It's confusing to have a test pass that says the button is disabled, while also allowing the click event go through.
Original issue in our repo: vmware-clarity/core#153

[ph-fritsche]

Could you break this down to a minimal example without third-party utils?

The implementation in jest-dom isn't sufficient for this library. The behavior ensured in the following test is also observed in browsers.

user-event/tests/utils/misc/isDisabled.ts

Lines 67 to 71 in 1aa2027

	'other custom element': {
	html: `<custom-el disabled></custom-el>`,
	node: '*',
	expected: false,
	},

[ashleyryan]

It would take a lot of code to create a reproduction of our button element - especially because we use Lit to create our components, which I'm sure you also wouldn't want in a repro case. Let me see what I can come up with.

I saw your tests, but in practice, it's so much more complicated than simply setting formAssociated and my argument is that that's overly restrictive in a world where browsers don't properly support ElementInternals yet. Using formAssociated without attachInternals() is probably okay, but might cause problems if the Elementnternal API isn't being implemented in the custom component.

Browsers don't follow that same behavior though - there are so many ways to prevent an element from being clickable that aren't simply setting formAssociated. Sure, your contrived test matches the spec, but in a world where browsers don't yet properlty implement the spec, it's not an accurate test. In practice, are there any custom elements that want click behavior when disabled is set on it?

[ashleyryan]

Ok, this is a pretty contrived example, but I've found a lot of design systems use a button element in the shadow dom for their custom button behavior. That's essentially what ours does as well, though ours is hidden.

In a perfect world, would every custom button have formAssociated and implement the form behavior using attachInternals? Yes, but because browsers still don't support attachInternals, years of workarounds have been made, and following the spec exactly here is overly restrictive. At this time, I'm not comfortable setting formAssociated on my element unless I'm also implementing attachInternals which we don't plan to do until the browser support is better.

However, I've realized that I can give our react consumers the following workaround. In their jest setup file (where we have to mock all the things jsdom doesn't implement), they can put the following:

(CdsButton as typeof CdsButton & {formAssociated?: boolean}).formAssociated = true;

It would be nice if they didn't have to do that though.

[ph-fritsche]

Before this issue sounds as being easily dismissed: Thanks for raising this issue! ❤️

Our goal is to provide a testing tool that helps to ensure code works and follows best practice. If something works in all major browsers, we generally will try to support it. If something doesn't work across different browsers, we'll most likely follow the spec (if it exists) or try to come up with a reasonable default.

It would take a lot of code to create a reproduction of our button element - especially because we use Lit to create our components, which I'm sure you also wouldn't want in a repro case.

I don't know how much code or what code it takes to make the browser consider this element disabled.
But I guess there has to be at least some common ground for major browsers if your workaround is valid. Otherwise we would consider it bad practice to rely on it and recommend to implement this according to specs (and maybe apply workarounds in browsers which don't follow these yet).

In practice, are there any custom elements that want click behavior when disabled is set on it?

While I see your point, this is something we can't consider. What if we break someone's test who does exactly this? Their component would match specs and work in the browser and our test would fail.

[ashleyryan]

While I see your point, this is something we can't consider. What if we break someone's test who does exactly this? Their component would match specs and work in the browser and our test would fail.

The previous implementation of isDisabled only checked for return Boolean(element && (element as {disabled?: boolean}).disabled) so if you went back to more closely match the v13 behavior, it wouldn't be a breaking change. No one reported the old behavior as being an issue for their web component.

I tried checking with a few other web component components libraries, but was having trouble getting the tests to run. The Shoelace.Style button now clicks when disabled as well.

I forgot to link the sandbox with a repro case in my previous comment, oops: https://codesandbox.io/s/hopeful-hugle-c5xqcg

[ph-fritsche]

if you went back to more closely match the v13 behavior, it wouldn't be a breaking change. No one reported the old behavior as being an issue for web components.

We won't consider breaking with Semver rules here. If something is released and isn't considered a bug, it won't change. We can't introduce a potentially breaking change just because someone relied on it without filing an issue.

You've got a case that your component seems to work in browsers as described. If this behavior is consistent, we'll try to support it. But we need to know what exactly triggers this behavior or we'd just be (re)introducing a bug.

[ashleyryan]

I'm not sure if you saw the link I posted above, but I didn't realize I forgot to include a stackblitz in my second comment above.

There are a couple of ways of creating custom buttons to get them to interact and submit forms.

1. render a native button in the light or shadow dom that you either style, or hide, but will behave like a real button. In this case, the native button is the real button, and your custom element is simply a wrapper over it. So when a user clicks the custom element, they're really clicking the child button, which is disabled and therefore doesn't fire. This is more or less what both Clarity Core (though we don't render the button when disabled) and Shoelace Style Web Components do. Aurora, Alaska Airline's Design System does the same thing.

2. Don't use a real button at all, simply capture the click event on yourself during the capture phase, and call stepImmediatePropagation if your element is disabled. This is how Adobe's Spectrum web components do it. I can't get their web components to run in jsdom to test it, but I suspect theirs would be broken as well. Here's a repro of that: https://codesandbox.io/s/magical-glitter-6xu8ew?file=/src/App.js:660-685

There's probably other ways of doing it as well. For #1, you could attempt to look for a button in the shadowDOM, I suppose. But that's a hack and you obviously shouldn't be browsing the shadowDOM, it's supposed to be private. I don't know how you would catch #2.

I've looked through the implementation for 4 different web-component design system buttons - VMware's Clarity, Adobe's Spectrum, Shoelace.style, and Alaska Airline's Aurora and none of them set formAssociated or use attachInternals. I can't get most of them to run in jsdom - they either throw an exception about mutationobserver or an issue with dispatchevent.

[ph-fritsche]

So when a user clicks the custom element, they're really clicking the child button,

I'm not sure I can follow. Is the custom element the event target or not? If is isn't, user.click(theCustomElement) describes just the wrong target.

simply capture the click event on yourself during the capture phase, and call stopImmediatePropagation if your element is disabled. [...] I suspect theirs would be broken as well

Again I can't follow. For capturing and stopping propagation there has to be an event. Why should this be broken by dispatching the event that is supposed to be captured?

[ashleyryan]

I'm not sure I can follow. Is the custom element the event target or not? If is isn't, user.click(theCustomElement) describes just the wrong target.

It uses event bubbling to propagate the event. You can't target the inner button because it's essentially part of a private API. For all intents and purposes, the custom element is the button, it has role="button" and that's where you apply the click handler. The wrapped button is an implementation detail, used to get form submission (since Safari doesn't implement attachInternals() yet)

Again I can't follow. For capturing and stopping propagation there has to be an event. Why should this be broken by dispatching the event that is supposed to be captured?

In this second use case, imagine the custom element is simply a div with an attribute called disabled on it. To get proper disabled behavior, I (the custom element author) capture the event in the capture phase, check if the button is disabled, and stop it from propagating any further. I do not want the click event to fire any further since you shouldn't be able to click.

As a third option, there's also pointer-events: none but I can't image any custom element author would try and stop click behavior through CSS alone.

In any case, I'm not sure how to recommend supporting this use case since there are so many workarounds to prevent an element from being clicked. Perhaps you'll be willing to consider a breaking change for v15 that isn't so restrictive, or perhaps Safari will have it implemented by then and all of the custom element authors will be using formAssociated. Until then, I'll recommend to our React consumers to add the formAssociated static property to the button class in their jest setup file.

[ph-fritsche]

In this case, the native button is the real button, and your custom element is simply a wrapper over it. So when a user clicks the custom element, they're really clicking the child button,

You can't target the inner button because it's essentially part of a private API. For all intents and purposes, the custom element is the button,

I think this might be a misconception. If there are two elements in the document, they are two different event targets.

If the wrapped button is the uppermost layer that can receive pointer events it is the event target for a pointer event.
The browser will dispatch the events there and they are propagated from there. If it is disabled, this prevents mouse events.

If this is the case, the wrapped button is the correct target for user-event. There is no way for user-event to know, that the button declared in the API is not the event target in the browser.

There might be a nicer way to describe such interaction in the future when #846 is implemented, but this will require running the test in an environment with layout - e.g. a real browser.

and stop it from propagating any further. I do not want the click event to fire any further since you shouldn't be able to click.

The event dispatched by this library can be handled like any other.
Could you please explain the difference between the event dispatched by user-event and the respective event by the browser here?

Perhaps you'll be willing to consider a breaking change for v15 that isn't so restrictive

I'll consider any change if we can work out what exactly should be handled differently.
But we want to help authors to reduce technical debt. This requires us to be restrictive sometimes.

Until then, I'll recommend to our React consumers to add the formAssociated static property to the button class in their jest setup file.

If the target is incorrect, you should consider providing a helper to get the correct one.

function getButtonTarget(customEl) {
  return customEl.shadowRoot.querySelector('button')
}

await user.click(getButtonTarget(screen.getByRole('button')))

[ashleyryan]

Could you please explain the difference between the event dispatched by user-event and the respective event by the browser here?

It's not any different. The problem is that you dispatch it even the custom element is disabled because the custom element doesn't have formAttached.

But we want to help authors to reduce technical debt.

I'll reduce technical debt when the major browsers support attachInternals, which Safari doesn't currently.

If the target is incorrect, you should consider providing a helper to get the correct one.

I'm not providing a helper, I don't provide any helpers. The custom element is the correct target, it is the button, it has this.addEventListener('click') in it. The inner button is simply there to interact with the native form element, which I can't my custom element cannot do until attachInternals is supported.

[ph-fritsche]

It's not any different. The problem is that you dispatch it even the custom element is disabled because the custom element doesn't have formAttached.

If the custom element isn't formAssociated, it isn't disabled according to specs.
If browsers handle this differently, we would like to adjust the behavior of this library. But there is a test above for behavior which is correct in the browser.
So either there is some edge case that results in browsers handling the simple custom element in the test differently than the button in your example, or there isn't and the behavior is correct.

[ph-fritsche]

The custom element is the correct target.

I took your first example to verify this and Chrome thinks you're wrong.

https://codesandbox.io/s/keen-panini-g0vwmg?file=/src/index.js