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
fix: Update diffDOM to the latest version #7461
Conversation
@@ -49,7 +49,7 @@ | |||
}, | |||
"dependencies": { | |||
"babel-runtime": "^6.23.0", | |||
"diff-dom": "2.5.1", | |||
"diff-dom": "4.2.8", |
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.
Should we add it with ^
sign to allow upgrade for minor version in future in package-lock.json file?
@vinitkumar Do you have an idea why the javascript tests fail? |
@fsbraun @vinitkumar have you been able to get the bottom of the tests failing? |
So far, I have not succeeded. @FinalAngel Do you think you can take a look? |
@petrklus It seems I have closed this PR. I am not certain why and how. Also, I cannot reopen. Can you reopen this PR? |
@fsbraun I think it was closed because the branch |
OMG, I did not do this on purpose! It's restored now. Is there a particular reason you opened the PR against |
Description
Work in progress on #7460.
Upgrading diffDOM seems to resolve the issue, however, is not compatible with UglifyJS.
Related resources
The issue seems to be related to variable naming in diffDOM:
I belive it is due to the variable called
node
:https://github.com/fiduswriter/diffDOM/blob/v4.2.8/src/diffDOM/dom/fromVirtual.js#L2
Which UglifyJS does not like. I am seeking help with getting this fix across the finish line.
Checklist
develop