-
Notifications
You must be signed in to change notification settings - Fork 25
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
Disable setImmediate
event loop yielding in the browser,
#50
Conversation
Codecov Report
@@ Coverage Diff @@
## master #50 +/- ##
==========================================
- Coverage 99.53% 99.30% -0.23%
==========================================
Files 9 9
Lines 427 431 +4
Branches 68 69 +1
==========================================
+ Hits 425 428 +3
- Misses 2 3 +1
Continue to review full report at Codecov.
|
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.
Just a suggestion of changing the triggering behaviour. There's one more change you may want to throw in, if you don't mind;
Over in visitor.js
we can even stop visitLoop
from yielding in the first place by adding SHOULD_YIELD &&
to this if-statement, which is what interrupts the render-loop:
react-ssr-prepass/src/visitor.js
Lines 192 to 194 in 9f0fde2
if (Date.now() - start > YIELD_AFTER_MS) { | |
return true | |
} |
Co-Authored-By: Phil Plückthun <phil@kitten.sh>
@kitten Applied suggestions from code review |
@kitten I'm really not really sure where to move the |
@zenflow given that this is just a two occurrence constant I wouldn’t be too bothered either redefining it or just exporting it from This way we don’t have to whack in a constants file for a single shared variable 😅 |
Done in fc814e2 |
Closes #48
To define
isServer
we can't use a simpletypeof window === 'undefined
because Jest provides some kind of shim forwindow
, and we want to run the tests in node.js conditions.