Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App renders twice #1330

Open
marlonmarcello opened this issue Jul 18, 2020 · 17 comments
Open

App renders twice #1330

marlonmarcello opened this issue Jul 18, 2020 · 17 comments

Comments

@marlonmarcello
Copy link

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 #457 breaks pre-rendering because there is no document.

Issue #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
@developit
Copy link
Member

developit commented Jul 21, 2020

@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.

@marlonmarcello
Copy link
Author

tell Preact CLI where to pick up on the client

Sorry @developit what did you mean by that?

@developit
Copy link
Member

developit commented Jul 21, 2020

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:

let root =
document.getElementById('preact_root') || document.body.firstElementChild;

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>

@marlonmarcello
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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

@marlonmarcello
Copy link
Author

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.

@developit
Copy link
Member

developit commented Jul 22, 2020

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!)

@marlonmarcello
Copy link
Author

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

@jdeagle
Copy link

jdeagle commented Jul 24, 2020

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?

@developit
Copy link
Member

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).

@rinatkhaziev
Copy link

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).)

@phulin
Copy link

phulin commented Sep 16, 2020

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.

@prateekbh
Copy link
Member

can you try preact watch --refresh

@phulin
Copy link

phulin commented Sep 16, 2020

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.

@JoviDeCroock
Copy link
Member

@phulin preact watch --refresh is HMR

@zoltantoth-seon
Copy link

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..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants