Skip to content

Conversation

@silver83
Copy link

when looking for a react root element the function would look at the 'container.firstChild' attribute, which is a convenience for childNodes[0]. this might be non-html nodes (#text for example), causing hierarchies containing spaces between the container and child (react root element) to fail locating the react root elem. modified to '.firstElementChild', which is a similar convenience for children[0]

…'container.firstChild' attribute, which is a convenience for childNodes[0].

this might be non-html nodes (#text for example), causing hierarchies containing spaces between the container and child (react root element) to fail locating the react root elem.
modified to '.firstElementChild', which is a similar convenience for children[0]
@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@sophiebits sophiebits changed the title when looking for a react root element the function would look at the 'container.firstChild' attribute, which is a convenience for childNodes[0]. this might be non-html nodes (#text for example), causing hierarchies containing spaces between the container and child (react root element) to fail locating the react root elem. modified to '.firstElementChild', which is a similar convenience for children[0] Use '.firstElementChild' when looking for root element Sep 10, 2014
@bloodyowl
Copy link
Contributor

careful, this isn't compatible with IE8

@heydenberk
Copy link

return container.children[0]; is compatible with IE6, 7 and 8, although it may erroneously return a comment node in those browsers: https://developer.mozilla.org/en-US/docs/Web/API/ParentNode.children

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@silver83
Copy link
Author

how would you feel about another shim ? is there already a place for such in the project ?

if (!('firstElementChild' in Element.prototype)) {
    Object.defineProperty(Element.prototype, 'firstElementChild', {
        'get': function() {
            for (var next = this.firstChild; next; next = next.nextSibling) {
                if (next.nodeType == 1)
                    return next;
            }
            return null;
        },
        'enumerable': false,
        'configurable': true
    });
}

@syranide
Copy link
Contributor

I personally recommend: #1912

@silver83
Copy link
Author

didnt fully understand #1912 - i cant have a newline or whitespace between the container and react root ? maybe understandable for Element nodes, but still kind of a weird limitation...

@syranide
Copy link
Contributor

@silver83 React clears the content of the element you render into when you render normally, so (I think) the same behavior should be maintained for reusing server-rendered markup. Also, if the client is for some reason unable to reuse the server-rendered DOM the content would again be completely cleared and rendered as normal (i.e. if we allowed whitespace there it could have affected rendering).

@silver83
Copy link
Author

hmm I understand your point - that approach simplifies the various cases of other 'siblings' in general.
Still feels like sending the developer back to remove the whitespace is a bit strict..

Up to you guys.

@syranide
Copy link
Contributor

Closing via #1912, thank you for your contribution anyways :)

@syranide syranide closed this Jan 31, 2015
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

Successfully merging this pull request may close these issues.

6 participants