search

gwuhaolin / chrome-render

general server render base on headless chrome
95 stars 10 forks source link

Use document event instead of console.log in useReady #13

Closed SkeLLLa closed 7 years ago

SkeLLLa commented 7 years ago

Hello again. As of crome-remote-interface API all console messages that were logged before are evaluated when Console.messageAdded method is called. So if you have the same chrome instance or tab on each render that's called it will increase those console messages which isn't very good.

I have a proposition to change setting the variable isPageReady to listening to event on document like:

document.addEventListener('crPageRendered', ...)

I've tested it locally and it seems that it's working much better that listening to console messages. It fires only once per render and doesn't produce console messages that could probably lead to potential memory leak (because for now there is no method for clearing them, the only option destroy chrome tab).

gwuhaolin commented 7 years ago

You are awesome, I've been looking for a good solution to replace Console.messageAdded way. Please make a merge request.

SkeLLLa commented 7 years ago

Ok, I'm cleaning up my code right now in separate branch and will make PR a bit later. But we'll probably need the https://github.com/gwuhaolin/chrome-pool/pull/7 to make it working. I'll test both options and write the results in PR.

@gwuhaolin and one question: should I leave window.isPageReady behavior for backward compatibility?

gwuhaolin commented 7 years ago

https://github.com/gwuhaolin/chrome-pool/pull/7 be added in https://github.com/gwuhaolin/chrome-pool/releases/tag/1.1.3

window.isPageReady should be compatibility for backward.

SkeLLLa commented 7 years ago

After testing and publishing to npm this issue and #8 could be closed.

gwuhaolin commented 7 years ago

Your code have some performance issues, I am reviewing code and make some change.

gwuhaolin commented 7 years ago

@SkeLLLa I optimize some of your code in https://github.com/gwuhaolin/chrome-render/commit/950cac860149b4a3193ca7be40e3d956682f8cb3 public in https://github.com/gwuhaolin/chrome-render/releases/tag/1.3.1