downshift
downshift copied to clipboard
Shadow DOM: Uncaught TypeError: documentProp.createElement is not a function
-
downshift
version: 3.4.2 -
node
version: 10.15 -
npm
(oryarn
) version: 6.4.1
Relevant code or config
We are using downshift in a component contained in a shadow DOM. We create the environment object used by downshift with the following code:
function createDownshiftEnvironment(shadowRoot: ShadowRoot): any {
if (!shadowRoot) {
return window;
}
return {
document: shadowRoot,
addEventListener: shadowRoot.addEventListener.bind(shadowRoot),
removeEventListener: shadowRoot.removeEventListener.bind(shadowRoot)
};
}
Note that this is different than the other examples I've seen where they set document: shadowRoot.ownerDocument
instead which I believe is a mistake. There are a few places in the library that uses getElementById
or activeElement
which in the context of a shadow root, should be from the shadow root, not the owner document. If we change it to ownerDocument
then getItemNodeFromIndex
no longer works properly.
The PR https://github.com/downshift-js/downshift/pull/755 introduced a call to createElement
which does not exist on the shadow root. The code in getStatusDiv
is arguably wrong because if the dropdown is operating in a shadow DOM, it really shouldn't be creating new elements inside the owner document but should be doing so below the shadow root.
What happened:
downshift.esm.js:355 Uncaught TypeError: documentProp.createElement is not a function
at getStatusDiv (downshift.esm.js:355)
at setStatus (downshift.esm.js:328)
at downshift.esm.js:1055
at downshift.esm.js:86
Suggested solution:
The code in getStatusDiv
should probably be changed so it can work either with a document or with a shadow root. And the environment object may need to be expanded so it includes the right hooks to support both environments. Right now, document
is wrong in some cases if you pass in shadowRoot
and it's wrong in other cases if you pass in shadowRoot.ownerDocument
.
Also, it would be good if the documentation could be expanded to make it clear what exactly is needed in the environment
prop. Right now the doc just links to another repo with an example I believe is wrong and the doc in this repo doesn't make it clear just which properties are required. The recent addition of using createElement
is a good example.
I do see this as a propTypes that is now missing createElement
. That could be useful to put in to the documentation.
environment: PropTypes.shape({
addEventListener: PropTypes.func,
removeEventListener: PropTypes.func,
document: PropTypes.shape({
getElementById: PropTypes.func,
activeElement: PropTypes.any,
body: PropTypes.any,
}),
}),
Workaround:
Our workaround was to add the following element at the top of our app (which mirrors the code in https://github.com/downshift-js/downshift/blob/master/src/set-a11y-status.js):
<VisuallyHidden id="a11y-status-message" role="status" aria-live="polite" aria-relevant="additions text" />
Where VisuallyHidden
is just a component that visually hides the element. This allows the downshift code to work.
Another possibility would be to modify the environment object so that it passes getElementByID, activeElement, body and createElement all as required by downshift.
Ok, this is just a case we did not considered, so we should fix it. Can you create a PR that makes getStatusDiv
take into account this case and also update documentation/propTypes? Also change it in useSelect please. Thanks for investigating this!
Covered in https://github.com/downshift-js/downshift/pull/1509