element.focus() not setting document.activeElement #2337
Comments
|
JSDOM is in full control of how The implication is that this changed in JSDOM v15; I'd suggest investigating their changelog. |
|
@ljharb thanks for the response, I think you're right but is to add |
|
That enzyme can be used to work around a jsdom issue doesn’t change the source of the issue :-) |
|
thank you for this! just had the very same issue. |
|
Thread necromancy but I thiiink this might be an issue in enzyme after all.
Doing a bit of investigation, I think jsdom has gotten stricter and more in-line with what browsers do between v14 and v16 that Jest v25 uses. I think jsdom is in the right here - it matches browser behaviour, and that enzyme will need to mount elements slightly differently if it wants to have Consider the following JSBin: https://jsbin.com/tuqonikavu/edit?js,console,output It shows that in a browser:
Looking at the react adaptor it looks like the element that you create to mount an element onto by default is That sounds like the current enzyme behaviour using jsdom v16 given the "default" behaviour and @vire's workaround using To get this working out-the-box I think Enzyme will want to mount attach content somewhere under the |
|
@BPScott |
|
Yep that'll solve the issue in userland. However I get the feeling that this is going to trip a lot of folks up. Personally, the team I'm on has written several tests that help ensure accessibility - that focus is moved to a particular element at a given element at a point in time, and I did not expect these to require modifications after a jsdom update. I expected that when I mounted elements that they'd be mounted within the body of a document and thus focus changing would work like when I have visible elements within a browser - do my manual checking with elements attached to a body I would have thought my automated tests would work in the same way by default. I think we want to encourage people to write tests around focus management, and making it so they have to add Low-effort userland solution:This is subtly buggy - if the expect check fails then then it('should focus', () => {
const ele = mount(<button>Hi</button>, { attachTo: document.body });
ele.focus();
expect(document.activeElement).toBe(ele.getDOMNode());
// Clean up
ele.unmount();
});Bulletproof userland solution:This fixes the "expect failures will cause leakages into other tests" problem but it is a lot more boilerplate. I'd wager most people won't be aware of this subtlety and go for the lower-effort solution mentioned above describe('my thing', () => {
let ele;
afterAll(() => {
ele && ele.unmount();
})
it('should focus', () => {
ele = mount(<button>Hi</button>, { attachTo: document.body });
ele.focus();
expect(document.activeElement).toBe(ele.getDOMNode());
});
})Ideal end state, this currently does not work because mount does not attach things to the body by default: it('should focus', () => {
const ele = mount(<button>Hi</button>);
ele.focus();
expect(document.activeElement).toBe(ele.getDOMNode());
}); |
|
I'm happy to pursue a better solution - I'm not sure what it would be, though (note, |
Ah yes, sorry - have updated those references to |
|
Enzyme could export a utility function that we would use like: import { buildEnzymeWrapper } from 'enzyme/jest';
describe('', () => {
const { mount } = buildEnzymeWrapper();
it('should focus', () => {
ele = mount(<button>Hi</button>);
ele.focus();
expect(document.activeElement).toBe(ele.getDOMNode());
});
});The Then the devs wouldn't be required to clean up the wrappers, nor remember about the |
|
For now, I've build myself a simple function like this: /**
* If you want to have your element to be attached to `document.body`
* (which is needed for example when you want to test focusing by referencing `document.activeElement`),
* use this to build a container element
* that can be later passed to the `mount()` function as `attachTo` param.
*
* Usage:
* ```tsx
* describe('Button', () => {
* const attachTo = buildMountAttachTarget();
*
* it('works', () => {
* const output = mount(<Button />, {
* attachTo,
* });
* });
* });
* ```
*/
export function buildMountAttachTarget() {
let container: HTMLDivElement;
beforeEach(() => {
container = document.createElement('div');
document.body.append(container);
});
afterEach(() => {
document.body.removeChild(container!);
});
return container!;
}However it would be very nice to have that built-in in the Enzyme library. (Honestly, I think I wrote something similar to it for the 3rd time already, each time in a different project.) |
|
@jtomaszewski enzyme couldn't export that because then it'd be tightly coupled to a single test runner. All our docs are written in mocha + chai, and you can use enzyme with any test runner you like, or none at all - if enzyme required jest, then a lot of stuff would be easier, but not everyone uses jest. |
|
Yeah, I know this. But I'm sure we could have a separate package that everybody using both enzyme and jest could use. (Nowadays it seems like 90% of community is using jest; ignoring that would be ignoring that community.) Should I file such feature requests against other repository (or create my own)? I mean, we could have such helpers living at https://github.com/enzymejs/enzyme/tree/master/packages/enzyme-jest-helpers , for example. I could help developing that a bit. |
|
That’s be a fine project to explore. |
|
In case it can help someone, this is what I did in our project: In Jest setup file:
// setupJest.client.js
/**
* In order to attach a mounted component to a div element we have to create
* and remove an element from the node before and after each test.
*/
let container: HTMLDivElement | null;
beforeEach(() => {
container = document.createElement("div");
container.id = "enzymeContainer";
document.body.appendChild(container);
});
afterEach(() => {
if (container && container.parentNode) {
container.parentNode.removeChild(container);
}
container = null;
});Then I created a custom wrapper for mounting, using Enzyme mount, like so: // jest/containers.ts
import { mount as enzymeMount } from "enzyme";
export const mount = (component: ReactNode) =>
enzymeMount(component, {
attachTo: document.getElementById("enzymeContainer")
});Usage: // exampleTest.ts
import { mount } from "./jest/containers.ts";
describe('', () => {
it('should focus', () => {
ele = mount(<button>Hi</button>);
ele.focus();
expect(document.activeElement).toBe(ele.getDOMNode());
});
});This approach works very well, and we are using it with around 4k of tests, without any issues. The only drawback with this is that you can have only one mounting per test, so be aware of it. I am happy to provide more details should you need it. |
Current behavior
After explicitly calling
.focus()on DOM Node under testdocument.activeElementswitches from<body>tonullExpected behavior
After explicitly calling
.focus()on DOM Node under testdocument.activeElementswitches to DOM Node under testYour environment
NOTE: this was working with
testEnvironment: 'jest-environment-jsdom-fourteen',API
Version
Adapter
The text was updated successfully, but these errors were encountered: