Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Tracking Issue: Stress-testing Directives Hydration 🧪 #92

Closed
michalczaplinski opened this issue Oct 24, 2022 · 6 comments
Closed

Tracking Issue: Stress-testing Directives Hydration 🧪 #92

michalczaplinski opened this issue Oct 24, 2022 · 6 comments

Comments

@michalczaplinski
Copy link
Collaborator

michalczaplinski commented Oct 24, 2022

This issue tracks the work done to test the performance and resilience of the Directives Hydration mechanism. As this approach requires building up a representation of the DOM of the whole page, we have to make sure that it performs well and does not interfere with third party scripts and styles.

The main branch of this experiment is main-wp-directives-plugin. Make sure you select it when opening a pull request if it is related to this experiment. Note that this is the same branch that we're using to develop the WordPress Directives Plugin!

Please proactively comment on this issue whenever you start working on a new task, get blocked, or finish a task, sharing as much detail and information as possible. Thanks!!

@michalczaplinski michalczaplinski changed the title Tracking Issue: Performance and stress-testing vDOM Hydration Tracking Issue: Performance and stress-testing vDOM Hydration 🧪 Oct 24, 2022
@michalczaplinski
Copy link
Collaborator Author

I've started work on adding some performance and stress tests:

@DAreRodz
Copy link
Collaborator

I've created an experiment using a proxy to inject our hydration scripts to automate Lighthouse testing.

@cbravobernal
Copy link
Collaborator

I'm working on a script to get a post RUL to stress test for the list of sites.

@SantosGuillamot
Copy link
Contributor

SantosGuillamot commented Jan 10, 2023

I already shared this in its Pull Request, but I'm sharing it here as well, as it is relevant.

We have tested all the sites and solved all the issues we encountered. We have been testing that the hydration scripts work fine in those sites, that they don’t break, and that we aren’t modifying the DOM. We haven’t covered performance testing in this first batch.

Here's a summary and a recap of the process. Looking at the results, I believe we should feel confident enough for this initial phase.

Results

Result Count % Info
Success 145720 83.79 Everything went fine
Cloudflare 21165 12.17 Skipped to avoid Cloudflare banning on our IPs
Timeout error 3080 1.77 Skipped because the request timed out
Error 3025 1.75 Skipped because the request didn’t return a valid response
403 error 929 0.53 Skipped because the request returned a 403 (forbidden)
Hydration error 0 0 Directives Hydration algorithm failed
Mutations 9 0 Directives Hydration algorithm produced DOM mutations
Total 173919 100 Total sites tested

Main points

This is a list of the main issues we faced and the solutions proposed. We also iterated our testing script many times, but I’m not sharing anything related to that here.

  • Remove comments: Preact doesn’t support them during the hydration so we created this Pull Request to remove them. We may need to add them back after the hydration if sites rely on them.
  • Nigloland.fr site issue: This site is using a script that detects the removal of the comments with DOMSubtreeModified and makes extra mutations that are detected by our MutationObserver. We shouldn’t worry about it, but ideally we should preserve the comments.
  • The less we rely on native methods, the better: 24life.ro modifies String.prototype.startsWith, which caused some mutations. As Luis suggests in the comment, the less we rely on native methods, the better. This was solved in this pull request.
  • CDATA nodes: We hadn’t taken into account the CDATA nodes ( nodeType 4), so they were returning some errors and registering mutations. As David explains here, they are not allowed in HTML documents, but they are used inside SVGs. To solve it, we are converting them into Text nodes, implemented in this pull request.
  • xpacket nodes: Similar to the previous point, we hadn’t yet considered the ProcessingInstruction nodes (nodeType 7), and it was returning a hydration error. Again, it is not HTML, but it can be found in XML elements like SVGs. We are treating them like comments, implemented in this pull request.
  • ref attributes: We also encountered that many sites were returning an error when there was a ref attribute in the content. At the end, we decided to ignore them while generating virtual nodes.

@luisherranz luisherranz changed the title Tracking Issue: Performance and stress-testing vDOM Hydration 🧪 Tracking Issue: Stress-testing vDOM Hydration 🧪 Jan 10, 2023
@luisherranz
Copy link
Member

Awesome work, folks 🙂👏

@SantosGuillamot
Copy link
Contributor

I believe we can consider this initial testing phase completed and successful, so I'm closing the issue. Great work, everyone ☺️

We haven't finally tested the performance, but after merging #124, I wouldn't consider it a priority right now.

@SantosGuillamot SantosGuillamot changed the title Tracking Issue: Stress-testing vDOM Hydration 🧪 Tracking Issue: Stress-testing Directives Hydration 🧪 Jan 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants