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

Update README about more Vuejs SSR hydration caveats #142

Closed
maxmilton opened this issue Feb 18, 2018 · 5 comments
Closed

Update README about more Vuejs SSR hydration caveats #142

maxmilton opened this issue Feb 18, 2018 · 5 comments

Comments

@maxmilton
Copy link

maxmilton commented Feb 18, 2018

When using react-snap with vue and webpack + vue-loader there's an extra client side hydration caveat worth adding to the readme.

When using react-snap with the minifyHtml.collapseWhitespace = true option vue client side hydration will fail because, by default, vue-loader includes whitespace when creating its virtual DOM representation. Since the VDOM and the real DOM are different (VDOM has extra whitespace textNodes), hydration fails.

Took me a while to work out what was going on but it turns out there's an easy solution. In the vue-loader configuration just add preserveWhitespace: false. In the official templates this is found in the vue-loader.conf.js file. Info in the vue-loader docs.

If that doesn't make sense please let me know and I'll try to explain better or create an example repo.

@maxmilton
Copy link
Author

maxmilton commented Feb 19, 2018

Update:
Vue uses empty comments as a marker when using v-if (<!----> when v-if == false or as a placeholder for async components/values). We need these empty comments in place so you need to make sure reactSnap.minifyHtml.removeComments is false.

Unforchunately this means you need to also implement your own comment removal logic which bloats the JS bundle a little. Example main.js with my normal window.snapSaveState function:

...

window.snapSaveState = () => {
  // enable client-side hydration once the page has been prerendered
  document.querySelector('#app').setAttribute('data-server-rendered', 'true');

  // remove scripts added to head by webpack; fix async race condition issue
  const scripts = document.head.getElementsByTagName('script');
  for (let i = 0; i < scripts.length; i += 1) { // forEach didn't work for some bizarre reason
    const el = scripts[i];

    if (el.async && el.charset === 'utf-8') {
      // console.warn('REMOVING:', el.src); // eslint-disable-line no-console
      el.remove();
    }
  }

  // remove comments (react-snap config can't take RegExp so do it custom)
  const html = document.documentElement;
  const noComments = html.innerHTML.replace(/<!--(?!\[if|-->)[\s\S]*?-->/g, '');
  html.innerHTML = noComments;
};

I've made a separate issue about allowing a JS config which would allow using reactSnap.minifyHtml.ignoreCustomComments = [/^$/] instead of the custom comment removal implementation.

@maxmilton maxmilton changed the title Update README about Vuejs SSR hydration caveat Update README about more Vuejs SSR hydration caveats Feb 19, 2018
@stereobooster
Copy link
Owner

stereobooster commented Feb 19, 2018

forEach didn't work for some bizarre reason

because

The Element.getElementsByTagName() method returns a live HTMLCollection...
The HTMLCollection interface represents a generic collection (array-like object similar to arguments)

To use forEach you need to convert HTMLCollection to array.

remove scripts added to head by webpack; fix async race condition issue

This case should be handled by react-snap. If it is not the case for you, please open an issue.

Thanks for tips. I'm not an expert in Vue. I want to experiment with it a bit and add notes to Readme

@maxmilton
Copy link
Author

Normally when dealing with a NodeList or HTMLCollection I do something like [...HTMLCollection].foreach(... but in my experimenting here it didn't work. I've seen some discussion about a babel transpile issue but it was easier to just use a good old loop. Anyway it's beside the point of this issue thread 🙃

I've opened another issue for the webpack bundle issue: #145

@maxmilton
Copy link
Author

Once I have some free time I'll try to put together a PR with the proposed docs updates.

@stereobooster
Copy link
Owner

Fixed in #172

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

2 participants