Skip to content
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

Recursion error logging React children (with hooks) #19

Closed
koenbok opened this issue Apr 15, 2019 · 9 comments
Closed

Recursion error logging React children (with hooks) #19

koenbok opened this issue Apr 15, 2019 · 9 comments

Comments

@koenbok
Copy link

koenbok commented Apr 15, 2019

Thanks for the great library!

I seem to have ran into an eccentric recursion issue.

You can quickly notice the page slowing down. When you inspect the log you get a result like this:

image

cc @CompuIves CodeSandbox has this too!

@samdenty
Copy link
Owner

samdenty commented Apr 15, 2019

Serializing logs with the current method is expensive, so when you call console.log, instead of sync blocking until the log has been dispatched, it delays it using setTimeout. Adding multiple todos, makes it block for longer.

Internally, replicator is currently used to serialize objects (so they can be transferred between DOM's), those stack frames are from within it.

The alternative way to handle recursion to use a two-way COM, which serializes objects as you expand them (the way chrome devtools does).

I haven't done this, as I've previously managed to get chrome devtools working embedded on a website using a mocked version of the devtools websocket API - giving you the real devtools, but I haven't put much time into that.

I don't currently have time to work on either, but pull requests are open

@samdenty
Copy link
Owner

samdenty commented Apr 15, 2019

You should be able to avoid the serializing / deserializing step, if the iframe is same-origin.

@koenbok
Copy link
Author

koenbok commented Apr 16, 2019

In our own app (Framer) we get this error too, but there console-feed just lives in the same page (so no iframe like CodeSandbox). Any hints how we could avoid this error in that setup?

@samdenty
Copy link
Owner

const encoded = Encode(parsed) as Message

Encode function should be removed

@koenbok
Copy link
Author

koenbok commented Apr 18, 2019

Would you be interested in a PR to make this an option? Or should we fork the library for our use case?

@samdenty
Copy link
Owner

samdenty commented Apr 18, 2019

@koenbok PR would be ideal, something like Hook(console, cb, encode = true)

@nvh
Copy link
Contributor

nvh commented Apr 19, 2019

@samdenty I just created a PR for this ^

samdenty pushed a commit that referenced this issue Apr 19, 2019
@samdenty
Copy link
Owner

samdenty commented Apr 19, 2019

Available in 2.8.8

@koenbok
Copy link
Author

koenbok commented Apr 21, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants