Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReactDOMRe.render's signature is too specific #50

Closed
glennsl opened this issue Feb 24, 2017 · 1 comment
Closed

ReactDOMRe.render's signature is too specific #50

glennsl opened this issue Feb 24, 2017 · 1 comment

Comments

@glennsl
Copy link
Member

glennsl commented Feb 24, 2017

It needs to accept either Node, or (DocumentFragment | Element), depending on what it actually needs (more likely the latter).

Discovered when trying to fit ShadowRoot into render, which isn't an Element but works perfectly fine with render if it pretends to be.

Also checked TypeScript's bindings, and they are equally wrong on this point. They do accept null though...

@glennsl
Copy link
Member Author

glennsl commented Mar 19, 2017

Had a look at react to figure out what ti actually supports. Doc comments say the type is DOMElement, but according to the code that's clearly incorrect. This is the actual check: https://github.com/facebook/react/blob/master/src/renderers/dom/stack/client/ReactMount.js#L235-L240

This is the PR that added it: facebook/react#3169
And there's some dicussion here: facebook/react#840

I suggest adding the overloads renderToDocument and renderToDocumentFragment post-reasonjs-upgrade.

@glennsl glennsl closed this as completed Mar 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant