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

Preact recreates and replaces root DOM element upon hydration instead of attaching itself to it #1471

Closed
fagyikaa opened this issue Nov 11, 2020 · 11 comments

Comments

@fagyikaa
Copy link

Do you want to request a feature or report a bug?
Bug
What is the current behaviour?
Preact rerenders the whole app and replaces the prerendered root node instead of attaching itself to it which causes a flick.
If the current behaviour is a bug, please provide the steps to reproduce.
Created a repository with a subset of my project: https://github.com/fagyikaa/preact-cli-critter-issue
Initially created it to demonstrate critter bug but it's demonstrates this bug as well.
Run npm build and npm serve (or just serve the already built code). When Preact starts after the initial page load you can see a flick.

What is the expected behaviour?
It should attach itself to the root node of the app and doesn't replace DOM nodes.

Please mention other relevant information.
To check my hypothesis edited the prerendered index.html and logged out the root node like this:
<body><div id="app"><script>console.log(document.querySelector('#'app))</script>...
It then printed out this div into console and when want to reveal in elements panel the console warns me that 'Node cannot be found in the current page.'
Also logged out canHydrate variable in preact-cli/lib/lib/entry.js which is true so hydrate is indeed called and not render.

Environment Info:
System:
OS: macOS 10.15.5
CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
Binaries:
Node: 12.18.3 - ~/.nvm/versions/node/v12.18.3/bin/node
npm: 6.14.6 - ~/.nvm/versions/node/v12.18.3/bin/npm
Browsers:
Chrome: 86.0.4240.183
Firefox: 80.0.1
Safari: 13.1.1
npmPackages:
preact: 10.3.2 => 10.3.2
preact-cli: ^3.0.0 => 3.0.0
preact-render-to-string: 5.1.4 => 5.1.4
preact-router: 3.2.1 => 3.2.1
npmGlobalPackages:
preact-cli: 3.0.1

@fagyikaa
Copy link
Author

fagyikaa commented Nov 11, 2020

Okay so i figured out that code splitting causes the problem (which is understandable) because the initial bundle doesn't contain the code of the requested page and it is in route-xy/index.jsx.chunk for each page. Disabling this rule in webpack-client-config.js:
rules: [ { test: /\.[jt]sx?$/, include: [ filter(source('routes') + '/{*,*/index}.{js,jsx,ts,tsx}'), filter( source('components') + '/{routes,async}/{*,*/index}.{js,jsx,ts,tsx}' ), ], loader: asyncLoader, options: { name(filename) { filename = normalizePath(filename); let relative = filename.replace(normalizePath(src), ''); if (!relative.includes('/routes/')) return false; return 'route-' + cleanFilename(relative); }, formatName(filename) { filename = normalizePath(filename); let relative = filename.replace(normalizePath(source('.')), ''); return cleanFilename(relative); }, }, }, ],

solves the problem however then it will generate an enormous bundle.js containing the code for each route.
Maybe separate bundles should be created for each page instead of a common bundle and index chunks loaded asynchronously? I know it's not the most optimised way of code splitting and it will contain more code this way (because the common bundle will be duplicated for each page) however the current behaviour causes a flickering which kills UX and also makes browsers repaint the whole app which cause very poor performance.

@rschristian
Copy link
Member

rschristian commented Nov 11, 2020

Your flickering issue is likely due to having app.jsx in routes/. You should take it out of there, it's not a route itself.

You're asynchronously loading the router which then asynchronously loads the page. On every navigation.

@fagyikaa
Copy link
Author

You're right, thank you for your help again!

@rschristian
Copy link
Member

(This one was easy as I was told not do this myself just a little while back on an unrelated problem)

Glad that's working for you now.

@fagyikaa
Copy link
Author

However switching between routes still creates the flickering. The initial page load is sweet now but navigating to an other route causes the effect. And it's smooth without any flick when i disabling async code splitting in config.

Extended my previous code with a new route: https://github.com/fagyikaa/preact-cli-critter-issue/tree/fagyikaa-patch-1
(Any menu elem in the navbar navigates to the other route.)

@rschristian
Copy link
Member

"Flickering" is quite vague, so this might just be the nature of the beast here. Are you seeing flickering in in the NavBar or footer, or is it just in the page content?

If you disable route splitting then yes, you don't get any real delay, but then you're sending a huge bundle to your clients. If your clients often use multiple pages, then great, this might work out for you, and you can disable. If they don't, however, then you're sending worthless JS to them. That's the balancing act.

@fagyikaa
Copy link
Author

The flickering is just the content and as i see it's caused because the page has to download the code of the other route. I see your point but I'm wondering why aren't the other route chunks preloaded? Is it worsening the points in pagespeed? E.g. NextJS adds preload link tags for each route which has a corresponding link in the DOM.

@rschristian
Copy link
Member

Quite frankly I don't know enough about preloading to speak about it, maybe hop into the slack if you'd like to speak with someone who does? I don't think this is an issue anymore.

If it's just the content then the route splitting is likely working as intended. Your repo seems to be setup correctly so (without checking it out) I'm going to guess that's just the UX price you pay for route splitting.

@fagyikaa
Copy link
Author

I think you're right and this UX price is necessary for faster page load with route code splitting however currently it's only preloading the requested route-s chunk and when switching onto an other route then it appends the required route's chunk as a link and downloads it on demand. This however can consume some time especially on a slower network that's why i think maybe preload link tags should be added for each route chunks. This wouldn't solve the flick of the content i think but would speed up route change.

@rschristian
Copy link
Member

Just now remembered back to this: #1367 (comment)

There's why they're not added.

@rschristian
Copy link
Member

Closing as answer was found. Let me know if you have any further questions.

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

No branches or pull requests

2 participants