-
Notifications
You must be signed in to change notification settings - Fork 47.5k
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
Give context aware message if markup reuse fails #2629
Conversation
return; | ||
} else { | ||
rootElement.removeAttribute(ReactMarkupChecksum.CHECKSUM_ATTR_NAME); | ||
var rootMarkup = rootElement.outerHTML; | ||
var diffIndex = longestCommonSubstring(markup, rootMarkup); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to blow up. n^2 storage where n is the length of the html. Going to be huge very quickly and you don't really need it. Can you just find the first character that differs instead and display [-20;+20] range from there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 816b011.
626e51f
to
816b011
Compare
* | ||
* @return {int} the index of the character where the strings diverge | ||
*/ | ||
function longestCommonSubstring(string1, string2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name doesn't reflect what it does anymore
The context is an extra message with comparison of where server and client markup started to differ.
bc494a5
to
a352b94
Compare
Give context aware message if markup reuse fails
If markup reuse fails, give context for the failure by showing where output from both server and client side markup start to differ: