preact-cli icon indicating copy to clipboard operation
preact-cli copied to clipboard

App renders twice

Open marlonmarcello opened this issue 4 years ago • 17 comments

Do you want to request a feature or report a bug? bug

What is the current behaviour? App renders twice if there is anything inside the body.

Steps to repro preact create default preact-test cd preact-test Add a div to the body of the template, like so:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
    <title><% preact.title %></title>
    <meta name="viewport" content="width=device-width,initial-scale=1" />
    <meta name="mobile-web-app-capable" content="yes" />
    <meta name="apple-mobile-web-app-capable" content="yes" />
    <link rel="apple-touch-icon" href="/assets/icons/apple-touch-icon.png" />
    <% preact.headEnd %>
  </head>
  <body>
    <div></div>
    <% preact.bodyEnd %>
  </body>
</html>

npm run build npm run serve

Note: During development, that div is replaced. Shouldn't either.

What is the expected behaviour? Should only render once.

Please mention other relevant information. We should have a way to choose where the app renders. The solution mentioned here https://github.com/preactjs/preact-cli/issues/457 breaks pre-rendering because there is no document.

Issue https://github.com/preactjs/preact-cli/issues/264 was marked as stale but is another one related to this.

Another problem that would be solved by being able to choose where the app renders is compatibility to old libraries that require HTML elements to be on the page as soon as they load. For example:

...head
  <body>
    <div id="id-for-lib-that-will-inject-stuff"></div>
    
    <!-- preact stuff -->
    <div id="render-app-here"></div>
    <% preact.bodyEnd %>
    
     <script src="lib/path/here.js"></script>
  </body>
</html>

preact info

Environment Info:
  System:
    OS: macOS 10.15.6
    CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
  Binaries:
    Node: 12.16.2 - ~/n/bin/node
    Yarn: 1.22.4 - ~/npm/bin/yarn
    npm: 6.14.4 - ~/n/bin/npm
  Browsers:
    Chrome: 84.0.4147.89
    Firefox: 77.0.1
    Safari: 13.1.2
  npmPackages:
    preact: ^10.3.2 => 10.4.6
    preact-cli: ^3.0.0-rc.6 => 3.0.0-rc.18
    preact-render-to-string: ^5.1.4 => 5.1.10
    preact-router: ^3.2.1 => 3.2.1
  npmGlobalPackages:
    preact-cli: 3.0.0-rc.18

marlonmarcello avatar Jul 18 '20 01:07 marlonmarcello

@marlonmarcello FWIW the solution from #457 can be tweaked to support prerendering:

import App from './components/app';
import { render } from 'preact';

let app;
if (typeof document === 'undefined') {
  app = App;
}
else {
  render(<App />, document.getElementById('my-div-id'))
}
export default app;

However doing so will disable hydration and ignore prerender data, so I'd recommend against it.

Instead, I'd recommend setting an id attribute on your app's root element from JSX to tell Preact CLI where to pick up on the client:

// src/components/app.js
export default function App() {
  // whatever your root (outermost) component is.

  return (
    <div id="preact_root">
      {/* everything else here as you would normally */}
    </div>
  );
}

The element needs to be whatever you see rendered into <body>, but it can be any kind of element.

developit avatar Jul 21 '20 16:07 developit

tell Preact CLI where to pick up on the client

Sorry @developit what did you mean by that?

marlonmarcello avatar Jul 21 '20 17:07 marlonmarcello

heh - yeah that was vague haha. When Preact CLI loads your app in the browser, it has to choose a DOM element as the root in order to hydrate your components. Hydration is a type of fast initial rendering where Preact can assume the existing HTML structure of the page is already the same as what your components are going to render, so it doesn't have to do any work to "diff" the DOM.

Here's how it finds the "root" element in your app's pre-rendered HTML: https://github.com/preactjs/preact-cli/blob/39cab7a3eadb3015cc256d05810a4f92ffb55d69/packages/cli/lib/lib/entry.js#L34-L35

With the default template, there will be no element with id="preact_root". In this case, we essentially make a "guess", and hydrate starting from the first Element in <body>:

<body>
    <div></div>    <!-- ⬅ preact assumes this is your app's rendered HTML -->
    <other-stuff-here>
    <script src="..blah"></script>
</body>

In your case, that first element is definitely not the pre-rendered <App/> component, so we're essentially "guessing wrong".

If you render your app's root JSX element with that id="preact_root" prop, we no longer have to guess which element within <body> was the pre-rendered HTML, since it's explicitly tagged as the root using an attribute:

<body>
    <div>this is not the App component's pre-rendered HTML</div>  <!-- ⬅ won't be touched -->

    <div id="preact_root">  <!-- ⬅ preact-cli will render (hydrate) your app using this element as the root -->
        <h1>hello from App.js</h1>
    </div>

    <script src="..blah"></script>
</body>

developit avatar Jul 21 '20 17:07 developit

Got it! Thank you for the explanation, really appreciate it. Just tested it and it doesn't seem to respect the order, or maybe I didn't quite understand. Here we go. Template:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
    <title><% preact.title %></title>
    <meta name="viewport" content="width=device-width,initial-scale=1" />
    <meta name="mobile-web-app-capable" content="yes" />
    <meta name="apple-mobile-web-app-capable" content="yes" />
    <link rel="apple-touch-icon" href="/assets/icons/apple-touch-icon.png" />
    <% preact.headEnd %>
  </head>
  <body>
    <h1>Tests</h1>

    <% preact.bodyEnd %>
  </body>
</html>

App.js

import { h, Fragment, Component } from "preact";
import { Router } from "preact-router";

import Header from "./header";

// Code-splitting is automated for routes
import Home from "../routes/home";
import Profile from "../routes/profile";

export default class App extends Component {
  /** Gets fired when the route changes.
   *	@param {Object} event		"change" event from [preact-router](http://git.io/preact-router)
   *	@param {string} event.url	The newly routed URL
   */
  handleRoute = (e) => {
    this.currentUrl = e.url;
  };

  render() {
    return (
      <div id="preact_root">
        <Header />
        <Router onChange={this.handleRoute}>
          <Home path="/" />
          <Profile path="/profile/" user="me" />
          <Profile path="/profile/:user" />
        </Router>
      </div>
    );
  }
}

When I run npm run dev the <h1>Tests</h1> is bellow the app. Screen Shot 2020-07-21 at 10 41 56 AM

marlonmarcello avatar Jul 21 '20 17:07 marlonmarcello

I also tried adding the <div id="preact_root"> to both, the template AND App.js but then when I run npm run build and npm run serve I see the app rendered twice. Screen Shot 2020-07-21 at 10 44 34 AM

marlonmarcello avatar Jul 21 '20 17:07 marlonmarcello

Let me know how I can help @developit or if more information is needed.

marlonmarcello avatar Jul 21 '20 17:07 marlonmarcello

I also tried adding the <div id="preact_root"> to both, the template AND App.js but then when I run npm run build and npm run serve I see the app rendered twice. Screen Shot 2020-07-21 at 10 44 34 AM

Just a note that in this instance, it renders fine in development, but in production that happens.

marlonmarcello avatar Jul 21 '20 18:07 marlonmarcello

Hmm - so your first example seems the most correct, but I had forgotten that in dev mode we don't do prerendering. Because of that, the CLI will always render the app as the first child of body.

It does seem like this is enough of a pain that we should be providing a more consistent solution. Ideally I would like to provide a way to specify the parent to render into, rather than the element to hydrate.

(I appreciate all your detailed debugging!)

developit avatar Jul 22 '20 04:07 developit

Of course! Love Preact. I wish I could be more helpful. Can a beginner on the framework tackle this issue?

marlonmarcello avatar Jul 23 '20 02:07 marlonmarcello

Running into the exact same issue. Need to pre-render into a particular part of the template and have the hydration pick up from that point. @developit should this still have the "has-fix" label?

jdeagle avatar Jul 24 '20 14:07 jdeagle

Changed the label to has-workaround since we have at least narrowed it down to a dev vs prod thing (there's no solution that works the same in both).

developit avatar Jul 24 '20 15:07 developit

I have stumbled upon this too, my solution was to use a custom template without<% preact.bodyEnd %>. I pretty much just copied what's in the body-end.ejs, but ensured that <%= htmlWebpackPlugin.options.ssr() %> is the first thing after the opening <body> and then I output the CMS stuff at the end.

	<body>
		<%= htmlWebpackPlugin.options.ssr() %>

		
		<script type="__PREACT_CLI_DATA__">
			<%= encodeURI(JSON.stringify(htmlWebpackPlugin.options.CLI_DATA)) %>
		</script>
		<% if (webpack.assets.filter(entry => entry.name.match(/bundle(\.\w{5})?.esm.js$/)).length > 0) { %>
			<% /* Fix for safari < 11 nomodule bug. TODO: Do the following only for safari. */ %>
			<script nomodule>!function(){var e=document,t=e.createElement("script");if(!("noModule"in t)&&"onbeforeload"in t){var n=!1;e.addEventListener("beforeload",function(e){if(e.target===t)n=!0;else if(!e.target.hasAttribute("nomodule")||!n)return;e.preventDefault()},!0),t.type="module",t.src=".",e.head.appendChild(t),t.remove()}}();</script>
			<script crossorigin="anonymous" src="<%= htmlWebpackPlugin.files.publicPath %><%= webpack.assets.filter(entry => entry.name.match(/bundle(\.\w{5})?.esm.js$/))[0].name %>" type="module"></script>
			<%
				/*Fetch and Promise polyfills are not needed for browsers that support type=module
				Please re-evaluate below line if adding more polyfills.*/
			%>
			<script nomodule src="<%= htmlWebpackPlugin.files.chunks["polyfills"].entry %>"></script>
			<script nomodule defer src="<%= htmlWebpackPlugin.files.chunks['bundle'].entry %>"></script>
		<% } else { %>
			<script <%= htmlWebpackPlugin.options.scriptLoading %>  src="<%= htmlWebpackPlugin.files.chunks['bundle'].entry %>"></script>
			<script nomodule src="<%= htmlWebpackPlugin.files.chunks["polyfills"].entry %>"></script>
		<% } %>
		<meta name="chunk" content="wp_footer">
	</body>
```

(`<meta name="chunk" content="wp_footer">` is the CMS part that gets replaced on backend later).)

rinatkhaziev avatar Aug 05 '20 01:08 rinatkhaziev

Hm, I've spent a while thinking about this but haven't been able to find a fix that works in development with webpack-dev-server's HMR. It renders correctly on first load, but then when HMR triggers it renders a second copy into the first child of body. Maybe I've missed the fix for this above, but I'm stumped here.

phulin avatar Sep 16 '20 12:09 phulin

can you try preact watch --refresh

prateekbh avatar Sep 16 '20 17:09 prateekbh

Thanks, that's a good workaround. I'd love to get HMR working, happy to try my hand at a PR if you can point me in the right direction. I just haven't been able to figure out why it's happening.

phulin avatar Sep 16 '20 18:09 phulin

@phulin preact watch --refresh is HMR

JoviDeCroock avatar Sep 16 '20 18:09 JoviDeCroock

I faced a similar issue using prerendering and the suggested workaround isn't working so i doubt this issue should be labeled with has-workaround as it won't be fixed then i guess. The main issue is that upon hydration (which is the case for prerendering) the root element is not used at all.

Preact's hydrate method simply doesn't accept third argument so it's useless if we pass the root (the element with preact_root id) as it will be simply ignored: https://github.com/preactjs/preact/blob/4aaecc18d33a9f3139ddd5f30f6d7c745076795f/src/render.js#L73

The only way i could overcome the problem is by wrapping preact.bodyEnd in the template into an element:

<body>
  <div id="someid">
	  <% preact.bodyEnd %>
  </div>
</body>

and then in the entry.js i pass as parent this element:

const doRender = canHydrate ? hydrate : render;
const hydrateParent = document.getElementById('someid');
root = doRender(h(app, { CLI_DATA }), canHydrate ? hydrateParent : document.body, root);

However forking preact-cli seems a very bad idea but idk how could i solve the problem other way..

zoltantoth-seon avatar Sep 27 '21 14:09 zoltantoth-seon