Place %IMPORTS% in body

monochromer commented 5 years ago

I think %IMPORTS%-block should be placed inside tag body. It will be more convenient.

vrizo commented 5 years ago

Sorry, I can't understand why. As far as I know it's OK to place imports in <head>.

monochromer commented 5 years ago

We render react apps when the dom is already loaded. With this in mind, in some components that work with portals, we create elements dynamically and place them in the body. Yes, we have added the render on event DomContentLoaded, but %imports% within body more reliable.

vrizo commented 5 years ago

in some components that work with portals

Could you please explain what does portals means?

we create elements dynamically and place them in the body

OK, but I can't figure out what's the problem with scripts and styles in <head>. It is not dynamically created. Uibook replaces %imports% while generating an HTML file. I don't think that another position in HTML can provide better reliability.

monochromer commented 5 years ago

About portals - https://ru.reactjs.org/docs/portals.html (It's useful for dropdowns, modals, tooltip, etc). In my components, that uses portals, I create container for portal and append in document.body which is not available because scripts in head.

vrizo commented 5 years ago

Hi, @monochromer !

I'm sorry for the long answer.

Thanks for the link! I checked the example in the manual and looks like scripts in <head> doesn't block portals. Here is an example: https://codepen.io/gaearon/pen/jGBWpE

monochromer commented 5 years ago

Your main scripts in document.body, not in document.head

vrizo commented 5 years ago

That's odd. As I can see, the special %SCRIPTS% tag is located in document.head — https://github.com/vrizo/uibook/blob/master/src/template.html#L9.

But I think it doesn't matter where it is located.

monochromer commented 5 years ago

Script of component is placed in document.head and i tried to get access to document.body for dynamicly creating dom-element and catch error because document.body doesn't exist on this moment

vrizo commented 5 years ago

Ah, OK, thank you, now I see. document.body exists before loading of any scripts. So the issue in dynamic DOM element which doesn't exist at the moment of rendering.

Moving <scripts> from <head> to <body> won't fix it globally. It might delay rendering, but on some computers it will fail anyway. It should be fixed in your component which uses createPortal().

According to the manual:

If a child component requires to be attached to the DOM tree immediately when mounted, for example to measure a DOM node, or uses 'autoFocus' in a descendant, add state to Modal and only render the children when Modal is inserted in the DOM tree.

So the proper way to fix the issue is to add state to your component, something like this:

class Modal extends React.Component {
  constructor(props) {
    super(props);
    this.el = document.createElement("div");
    this.state = { mounted: false };
  }
  componentDidMount() {
    modalRoot.appendChild(this.el);
    this.setState({ mounted: true });
  }
  componentWillUnmount() {
    if (this.el) {
      modalRoot.removeChild(this.el);
    }
  }

  render() {
    return ReactDOM.createPortal(
      this.state.mounted ? this.props.children : null,
      this.el
    );
  }
}

monochromer commented 5 years ago

or just move one line

vrizo / uibook

Place %IMPORTS% in body #6