react-modal icon indicating copy to clipboard operation
react-modal copied to clipboard

Failed prop type: Invalid prop `appElement` supplied to `Modal`.

Open moroshko opened this issue 3 years ago • 16 comments

Currently, appElement is expected to be an instance of window.HTMLElement.

But, when the Modal is rendered inside an iframe, the iframe's window.HTMLElement is a different instance causing the following warning:

Failed prop type: Invalid prop appElement supplied to Modal.

See: Codesandbox

What's the best to workaround this?

moroshko avatar May 14 '21 07:05 moroshko

Hi @moroshko, this is an interest case. Probably there is no workaround, because if we are dealing with 2 DOM/frame instances, I believe every property that expects an elements will fail.

Probably we need another strategy to validate these properties.

Any Idea?

diasbruno avatar May 14 '21 11:05 diasbruno

Is setting and removing attributes the only thing we do with the appElement? If so, what if we had a PropTypes function that checked whether appElement has a setAttribute (and maybe removeAttribute) methods?

moroshko avatar May 14 '21 12:05 moroshko

Perhaps node type or something...

diasbruno avatar May 14 '21 12:05 diasbruno

Do you mean PropTypes.node? It doesn't feel right, since it can be a number, for example.

moroshko avatar May 14 '21 12:05 moroshko

Something like...facebook/prop-types#proptypescheckproptypes

const IHTMLElement = {
  nodeType: PropTypes.number,
  setAttribute: PropTypes.func,
  removeAttribute: PropTypes.func,
  ....
};

const element: IHTMLElement = element;

const props = {
   appElement: element
};

PropTypes.checkPropTypes(IHTMLElement, element, 'prop', 'MyComponent');

or with custom props...

  customProp: function(props, propName, componentName) {
    if (props[propName].nodeType === x) {
      return new Error(
        'Invalid prop `' + propName + '` supplied to' +
        ' `' + componentName + '`. Validation failed.'
      );
    }
  },

diasbruno avatar May 14 '21 13:05 diasbruno

@moroshko I've pushed a branch here fix/appElement-property-detection...can you check if it fixes the problem, please?

diasbruno avatar May 14 '21 14:05 diasbruno

How could I try this branch in my app? It doesn't contain the lib folder which main in package.json points to. I get:

Can't resolve 'react-modal'

Could you publish a version to npm?

moroshko avatar May 15 '21 03:05 moroshko

@diasbruno Any chance to publish this fix to npm (even as a beta version)?

moroshko avatar May 17 '21 12:05 moroshko

@moroshko I fixed the reference for this test...you can use the same branch.

diasbruno avatar May 17 '21 20:05 diasbruno

@diasbruno I'm afraid it's not easy to test it the way you propose. I'm getting various errors like:

SyntaxError: Unexpected token

and

Support for the experimental syntax 'classProperties' isn't currently enabled

I think we need to compile it. Can we please publish a beta version to npm for testing? This way we are testing the real thing.

moroshko avatar May 17 '21 23:05 moroshko

Yeah...the project will need to have everything that react-modal uses to compile...I'll upload a precompiled version in this branch.

diasbruno avatar May 18 '21 00:05 diasbruno

Added the precompiled version. Hope now we can test this. 🤞

diasbruno avatar May 18 '21 23:05 diasbruno

The original warning has gone, but now we get:

Warning: Invalid argument supplied to oneOfType. Expected an array of check functions, but received an object at index 0.

moroshko avatar May 19 '21 00:05 moroshko

@diasbruno Any thoughts how to proceed from here?

moroshko avatar May 24 '21 01:05 moroshko

Has there been any update on this? I've just started running into the same issue too. Thanks!

andrewpye avatar Jan 27 '22 10:01 andrewpye

any updates?

doougui avatar Jul 26 '22 13:07 doougui