downshift icon indicating copy to clipboard operation
downshift copied to clipboard

Shadow DOM: Uncaught TypeError: documentProp.createElement is not a function

Open TazmanianD opened this issue 5 years ago • 1 comments

  • downshift version: 3.4.2
  • node version: 10.15
  • npm (or yarn) 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.

TazmanianD avatar Nov 25 '19 20:11 TazmanianD

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!

silviuaavram avatar Nov 27 '19 13:11 silviuaavram

Covered in https://github.com/downshift-js/downshift/pull/1509

silviuaavram avatar Jul 15 '23 12:07 silviuaavram