Skip to content

Resolve SSR Hydration Issue for Disqus plugin#1569

Merged
ang-zeyu merged 2 commits into
MarkBind:masterfrom
wxwxwxwx9:fix-disqus-hydration-issue
Jun 6, 2021
Merged

Resolve SSR Hydration Issue for Disqus plugin#1569
ang-zeyu merged 2 commits into
MarkBind:masterfrom
wxwxwxwx9:fix-disqus-hydration-issue

Conversation

@wxwxwxwx9

Copy link
Copy Markdown
Contributor

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain:

The hydration issue that Disqus plugin has is discovered in #1565. This PR will address the discovered hydration issue for Disqus plugin.

Overview of changes:
Processed <disqus> HTML element to be added with v-pre in NodeProcessor.

Anything you'd like to highlight / discuss:

  • Whether my approach will cause issues that I did not consider
  • Whether there is potentially other unknown HTML elements that can cause hydration issues.

Testing instructions:
Use Disqus plugin and insert <disqus/> HTML element in the page. Afterwards, serve the page by markbind serve -d. There should not be any warnings of hydration issue in the developer console.

Proposed commit message: (wrap lines at 72 characters)
Resolve SSR Hydration Issue for Disqus plugin.

<disqus> HTML tag is an unknown element to Vue as it is neither
a registered Vue component nor an official HTML element. This causes
hydration issue in our SSR setup whenever the Disqus plugin is used.

Let's add v-pre tag to <disqus> HTML tag so that Vue will ignore
the tag when compiling/rendering, thus resolving hydration issue.


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No blatantly unrelated changes
  • Pinged someone for a review!

@wxwxwxwx9 wxwxwxwx9 requested a review from ang-zeyu May 30, 2021 13:11
Comment thread packages/core/src/html/NodeProcessor.js Outdated
case 'style':
processScriptAndStyleTag(node, this.userScriptsAndStyles);
break;
case 'disqus':

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! How about just renaming the tag? So this fix lives strictly in the plugin

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ang-zeyu Thanks for the suggestion! We should really avoid unnecessary coupling. I've made the changes to avoid the coupling.

Regarding "renaming the tag", I'm not sure if we are thinking of the same thing. Let me know if you actually had another idea in mind!

@wxwxwxwx9 wxwxwxwx9 force-pushed the fix-disqus-hydration-issue branch from c0b83d5 to bbb1a41 Compare June 5, 2021 11:31
@wxwxwxwx9 wxwxwxwx9 force-pushed the fix-disqus-hydration-issue branch from 39cac78 to d2b74d6 Compare June 6, 2021 05:32

@ang-zeyu ang-zeyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm!

@ang-zeyu ang-zeyu merged commit 0565e95 into MarkBind:master Jun 6, 2021
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

Successfully merging this pull request may close these issues.

2 participants