search

DeloitteAU / react-habitat

⚛️ 🛅 A React DOM Bootstrapper designed to harmonise a hybrid 'CMS + React' application.
Other
262 stars 42 forks source link

Habitat Components Must Be Empty #5

Closed 6stringbeliever closed 7 years ago

6stringbeliever commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

Legend! Thank you..

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

jennasalau commented 7 years ago

Thanks for your PR Shaun, we really appreciate it!

This is now fixed and available in v0.4.0