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

Habitat Components Must Be Empty #5

Closed
6stringbeliever opened this issue Nov 21, 2016 · 4 comments
Closed

Habitat Components Must Be Empty #5

6stringbeliever opened this issue Nov 21, 2016 · 4 comments

Comments

@6stringbeliever
Copy link
Contributor

Is there a technical reason that habitat components must be empty?

I'm trying to be a good web citizen and progressively enhance my page. So for example, I want to add a react component that's an image gallery, but I'd like to have to have the featured image of the gallery load in the HTML from the server. So, I do this:

<div data-component="ImageGallery" data-r-prop-images="objectWithImages">
  <img src="featured-image.png" alt="Good web citizens also include alt text, right" />
</div>

This way, they get a useful page before the JS executes and it remains useful if the JavaScript borks for whatever of the myriad reasons JavaScript might bork.

This is actually working for me exactly as I want, but react-habitat is giving me console warning messages that say I can't do this.

I don't see anything in ReactDOM.render() that would make this problematic other than that it is destructive of the child nodes. But in this case, that's precisely the behavior I want.

PS: Thanks for the library. You saved my butt this weekend.

@jennasalau
Copy link
Contributor

Hey Shaun,

This is a valid point you raise.. Originally we put that in there because for some reason people felt the need to nest the DOM components rather than doing it in React. Nesting will cause a whole world of pain so we put this helper message in to stop it.

But i think your use case is an excellent idea and technically as long as there are no other nested components it is perfectly valid to have children in there like this.

Perhaps instead of showing this message blindly, we actually do a sneaky query on it's children looking for any child components and only then show the warning if found. What do you think?

Would you have any capacity to help us with a PR? It might take us a while to get to it.

Glad you like the framework though.

Jenna

@6stringbeliever
Copy link
Contributor Author

Cool. That makes perfect sense.

Yeah, should be pretty easy to just check for any children with a data-component attribute to determine whether to throw a warning. I'd be happy to put together a PR for this.

@jennasalau
Copy link
Contributor

Legend! Thank you..

Just make up a new warning code and ill add it to the wiki when we merge.

@jennasalau
Copy link
Contributor

Thanks for your PR Shaun, we really appreciate it!

This is now fixed and available in v0.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants