-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
rrdom #613
Conversation
@Yuyz0112 @eoghanmurray @Juice10 PTAL and make sure we are heading in the right direction. |
@Mark-Fenng cool, happy to see you already went down that road! |
@Mark-Fenng Does the one rrweb-snapshot use helps? |
@Yuyz0112 css doesn't support the mutual conversion between inline-style and cssText. |
@Mark-Fenng can you tell me about the virtual parent optimization, where in the current codebase is it? |
@eoghanmurray |
3e85e64
to
6a071ba
Compare
Errors are caused by the declaration similarity of @types/mocha and @types/jest if we install both of them in the whole project.
This mirrors the `Element.tagName` implementation: ``` For DOM trees which represent HTML documents, the returned tag name is always in the canonical upper-case form. For example, tagName called on a <div> element returns "DIV". ``` https://developer.mozilla.org/en-US/docs/Web/API/Element/tagName
Since Event class is built in nodejs after v15.0.0
7baada1
to
4793e94
Compare
I think I have mostly completed the content of stage 1. import { Replayer } from 'rrweb';
const fs = require('fs');
require('rrdom');
var events = JSON.parse(
fs.readFileSync(require.resolve('./events.json')).toString(),
); // Load rrweb events from a file.
var replayer = new Replayer(events, {
showWarning: false,
});
replayer.play();
replayer.on('finish', () => {
console.log("playback finished!");
}); I hope the current work could be merged into the master branch without being squashed. |
7be7a30
to
54b94f8
Compare
Absolutely loving this @Mark-Fenng! I'm reading through your PR, review will come soon! I noticed that |
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 honestly looks great! If we can get some of the duplication between the document-browser.ts
and the document-nodejs.ts
down I think this is ready to merge.
@Juice10 Your concern is absolutely reasonable. I split two versions of rrdom because rrdom for nodejs is getting bigger and it's not suitable for running on browsers. document-browser.ts is built for virtual dom optimization in rrweb replayer and it's in an initial state so that it's maybe not a proper time to do de-duplication work now. How about avoiding merging it into the main branch until it's finished and in a good code style? |
Purposes
Milestones