Wildhoney / ReactShadow

:beginner: Utilise Shadow DOM in React with all the benefits of style encapsulation.
https://react-shadow.herokuapp.com/
MIT License
1.31k stars 81 forks source link

Unable to use ReactShadow with Component #26

Closed mhuggins closed 5 years ago

mhuggins commented 7 years ago

I'm trying to create a higher order component for this, e.g.:

import React from "react";
import ShadowDOM from "react-shadow";

const withShadow = (...include) => (Component) => {
  return (props) => {
    return (
      <ShadowDOM include={include}>
        <Component {...props} />
      </ShadowDOM>
    );
  };
};

export default withShadow;

Usage:

const AppWrapper = _.flowRight(
  withStore(reducers),
  withShadow("path/to/app.css")
)(App);

ReactDOM.render(<AppWrapper />, ...);

Unfortunately I get an error:

ReactShadow: Passed child must be a concrete HTML element rather than another React component.

I don't see this documented, but I see that it's explicit in the source for this module. Is there a reason I can't use a component? Is it something that's possible to fix? I don't mind contributing, but I'm curious about the reason this is a limitation today.

mhuggins commented 7 years ago

FWIW, I was able to resolve this by throwing a <div> betweenand`, but not completely sure why it's needed. Might be good to add clarity in the README! Feel free to close this though.

Wildhoney commented 7 years ago

We have the check for passing a HTML node rather than a wrapped component, as ReactShadow needs to be able to determine the type of the node to append the shadow boundary to – otherwise it would have to recursively traverse the tree until it finds the first HTML node.

<div /> // this.props.children.type === 'div'
<WrappedComponent>
    <div /> // this.props.children.props.children.type === 'div'
</WrappedComponent>

We could do that, but it may have unforeseen consequences.

mhuggins commented 7 years ago

Is it because the shadow DOM function is called on the first HTML node it comes across? If so, how come the ShadowDOM component itself can't just create its own DOM element to us?

Wildhoney commented 7 years ago

Absolutely – we could do that, although it's making an assumption about the type of node to create, rather than taking it directly from your first child. I suppose we could accept an additional prop, such as wrapperNode.

Thoughts?

Wildhoney commented 7 years ago

I've taken a look at this again, and there are two issues, one of which we've spoken about:

I can achieve the former, but the latter I'm not sure about at the moment.

Does anybody have any suggestions?

javadoug commented 6 years ago

@Wildhoney regarding the finding the concrete child issue: First, I suggest making the error message clear to add an HTML DOM as first child of ShadowDOM so error gives fix, no need to search/waste time. Second, optionally add a shadowRoot prop that takes a String, or Node or whatever else you can accept as value.

nestarz commented 5 years ago

How can I include css from my Component without being ignored ? Is it possible ?

Wildhoney commented 5 years ago

Should work just fine with the updated version.