search

hybridsjs / hybrids

Extraordinary JavaScript UI framework with unique declarative and functional architecture
https://hybrids.js.org
MIT License
3.05k stars 85 forks source link

Styling a nested React component within a Hybrids component #53

Closed Avnerus closed 5 years ago

Avnerus commented 5 years ago

Hi! I am just beginning with Hybrids and so far enjoying it very much, but I ran across an issue regarding the integration with React. I am trying to use a React component inside my Hybrids layout. The component loads with the reactify method as in the react-counter example. However, I'm not sure what would be the best approach for styling the component. Let's say I get a CSS along with the 3rd party component that I can load or inject anywhere - I couldn't find how to inject this style to the component created by reactifiy. My best bet was to wrap the 'reactified'' component in another Hybrids component that also includes the style, and render the React component with shadowRoot: false. However, this for some reason breaks the handling of events in the React component. I was able to recreate the event issue in a react-counter example fork. Could you tip me on the best direction to proceed? Thank you for the work on this project! /Avner

Avnerus commented 5 years ago

Hi again! I was able to make some progress. This is how I integrated for example, react-flags-select:

import ReactFlagsSelect from 'react-flags-select';
import ReactFlagsSelectStyle from 'react-flags-select/css/react-flags-select.css';

function reactify(fn, style) {
  return (host) => {
    const Component = fn(host);
      return (host, target) => {
          const reactStyle = document.createElement('style');
          reactStyle.innerHTML = style;
          ReactDOM.render(Component, target);
          target.appendChild(reactStyle)
     }    
  }  
} 
const LanguageSelect =  {
    render: reactify(({count}) =>
        <ReactFlagsSelect />,
        ReactFlagsSelectStyle
    )   
}

It seems to work except there is some event retargeting issue that seems specific for that component. Is that a reasonable way to solve my problem?

Avnerus commented 5 years ago

As a followup on this issue, I just tried integrating react-select, and this one seems even more tricky. It uses emotion to generate the CSS classes on the fly and append them to <head>, which I guess is unusable for shadow dom? There might be some API that can change this behavior, but I'm not sure how this works with 3rd part components. Any ideas? For now my only option if I want to use React components seems to be declaring shadowRoot: false for all of my components. Thank you :)

smalluban commented 5 years ago

React components are not designed to work well in Shadow DOM. Like you wrote, styling might be the biggest problem and some event re-targeting.

The react counter example have used default settings of the render property to not over complicate example. However, I've changed it, and now it does not use Shadow DOM. You should avoid using it when wrapping React components.

The web component should be a bridge between React eco-system and web components. You can set properties and pass them to the component, so it then is easy to use react components with web components.

Look again at the react counter example, it is updated.

EDIT: In your example with ReactFlagsSelect styles should be applied to the document head, and only once. For now, they are appended to your custom element content every time when a render method is called. You don't have other properties to update component, but when they will be there, you can end with multiple style elements inside of every instance of your component (and those styles will be global).

Avnerus commented 5 years ago

Hi, Thank you for the insights! So if I understand correctly, if one wants to use React components in a Hybrids / Web components application, shadow root must be disabled all the way down the hierarchy to the React component? My question is then, can I still apply scoped CSS to my components when shadowRoot is off? I thought this might be handled by ShadyCSS, but I guess it's not active? I tried do it it here and there was no scoping.

Meanwhile I also stumbled upon a strange bug when shadowDom is off, but unfortunately I couldn't recreate it in StackBlitz, perhaps it relates somehow to Webpack, but maybe you will have some insight. I had a conditional element with `{shadowRoot: false} like so:

    render: render(({phase}) => 
        html`
        ${phase == PHASE.SIGN_IN && html`<sign-in></sign-in>`}
        ${phase == PHASE.MAIN && html`<h1>main phase!</h1>`}
        <div>
            <button onclick=${phaseClick}> Change phase </button>
        </div>
     `, {shadowRoot :false})

When I changed the phase I would get this error. It was trying to access some empty #text element where everything is null. However, if I add just one space after the sign-in element like this:

${phase == PHASE.SIGN_IN && html`<sign-in></sign-in> `}

or, if I changed the sign-in element to be shadowRoot: true. Then there is no error!

If you would like I can open another issue for this, but as I said unfortunately this doesn't happen in StackBlitz, and I really tried to reproduce the exact same environment..!

Thanks,

/Avner

smalluban commented 5 years ago

I did not consider the case when Shadow DOM is used in the parent custom element... Even though your react wrapper is not using shadow root, it will be inside of another one. Using wrapper in "light" DOM - on document level is safe then, but not inside of the other custom elements.

In that case, the only solution would be to switch back to Shadow DOM in the wrapper and pass styles into the shadow root boundary. It is not impossible, you could extend reactify() to allow to take additional styles argument, and create them only once (if they are not yet applied) and update react component with ReactDOM.render() method.

The problem with event propagation stays, so it might not be super easy to wrap complex react components into the web components after all :( It is not strictly related to hybrids as much as to Shadow DOM boundary.

About the bug - it would be great to have a reproduction of the error. If on StackBlitz it works, try on https://codesandbox.io. It looks that actually, something might be wrong. Make sure you're using the latest version and if the error occurs on all browsers or only on specific one (especially on those, which polyfills Shadow DOM, like IE11 or Edge).

smalluban commented 5 years ago

You were right! It was a bug... :( I already found it, and on the master branch, there is a fix: https://github.com/hybridsjs/hybrids/commit/4fb72fb9f0f5d3435aa50edd1a98672de1eac878

If all CI tests pass I will release a new version.

Avnerus commented 5 years ago

Hi! Thanks again for the response. Yeah, it seems traditional React components and Shadow Dom are not really meant to be... I think for my current small project I would try to just avoid react entirely.

I'm still interested about my previous question though - Providing I use shadowRoot : false for all of my components, and just use Hybrids as a functional templating engine that has the potential to use the shadow dom - how would you go about styling the components with scoping, as if ShadyCSS was being used? Is there a way to 'activate' ShadyCSS when using shadowRoot : false ? Shouldn't that actually be the standard behavior?

Great that you found the bug! Because I still couldn't reproduce it with codesandbox. Happy to contribute to Hybrids :)

smalluban commented 5 years ago

Yeah, it seems traditional React components and Shadow Dom are not really meant to be...

It depends. If the React component is isolated from the document (for example, not listening event on document level - then retargeting might be a problem), you can still inject styles into the Shadow DOM once for instance. Then it should work fine. React can be attached to shadowRoot - luckily it does not listen to events on the document level, but on the root level (to pass to own event system). However, you have to manually take care about ShadyCSS (built in template engine supports it out of the box), if you want to support older browsers.

If I find a moment, I will try to make a more functional react wrapper and I will post it here.

Great that you found the bug! Because I still couldn't reproduce it with codesandbox. Happy to contribute to Hybrids :)

v4.0.2 just has been released with bugfix. The bug was related to putting custom element (with render factory options set to shadowRoot: false) inside the nested template. The error occurs only if you remove the nested template in the sequential render, so thank you for discovering the problem!

Avnerus commented 5 years ago

Hi, Thanks for everything! I confirm the upgrade fixes the problem I described earlier. About React, well what made me give up, as I wrote earlier was dealing with react-select that uses emotion css, that dynamically creates styles and appends them to head.. Have you tried this though? https://github.com/spring-media/react-web-component

I'm not sure if you meant to answer my previous question:

Providing I use shadowRoot : false for all of my components, and just use Hybrids as a functional templating engine that has the potential to use the shadow dom - how would you go about styling the components with scoping

Did you have an advice for that? Thanks!

smalluban commented 5 years ago

react-web-component project might help, but it won't help with styles passing to head or problems with retargeting events - it is more a problem between Shadow DOM and how to react components are built.

Avoiding Shadow DOM at all will work. You will end up with a light DOM tree, as it would be created by react itself. However, the most powerful concept of web components is the Shadow DOM. It provides not only DOM and CSS encapsulation but also children distribution (by <slot> elements). The whole idea behind creating custom elements is to make them independent and not interacting with other elements. Putting styles in head breaks this promise.

Avnerus commented 5 years ago

Hi, thanks for the clarification. Yes I understand the advantages of Shadow DOM, in fact in the small project I am making I decided to just drop React entirely and use for example high-select as a select component. However I was thinking about the scenario in which hybrids could be used in a sort of transition period until all React components fully support Shadow DOM, or can be replaced by native components. In that case it would be nice to emulate the shadow dom and Shady CSS while still working in a light DOM, and later when it gets possible just turn on the Shaodw DOM back.

smalluban commented 5 years ago

It's good to hear you've dealt with the problem.

In that case it would be nice to emulate the shadow dom and Shady CSS while still working in a light DOM, and later when it gets possible just turn on the Shaodw DOM back.

The polyfill emulates Shadow DOM, but in the way, that all of your problems still exist. In another hand sometimes giving up on Shadow DOM is not possible, as its distribution and CSS encapsulation features are required.

The only way is to teach authors of the libraries to create code, which is more general and not only applies to a specific technology. It is just disappointing, but React is one of the worsts in supporting custom elements. They also have no strong intention to make changes on that soon (https://github.com/facebook/react/issues/11347).

As you moved out from React, it is not needed anymore to create a snippet of React factory (with ShadyCSS support). I'm closing issue, but feel free to re-open if you have any other concerns in this subject.

Avnerus commented 5 years ago

Thanks! And thanks again for making HybridsJS, I'm enjoying it very much.