-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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(flushMicroTasks): Fallback to no scheduler in case of exception #739
fix(flushMicroTasks): Fallback to no scheduler in case of exception #739
Conversation
@kentcdodds - I created this one following the conversation in #736, I still think as @eps1lon pointed out that we should talk about a proper solution since this sounds more like a band-aid than a way to fix the problem :/ |
As for whether we should not use the scheduler at all, I'd love to avoid it as well, but I know for a fact that without these changes, we run into some serious issues when using fake timers. I spent two days banging my head against the wall and this is the solution. I'd argue that |
@kentcdodds I agree with the approach and I sure think that this change is required. |
Yeah, luckily most of the time it shouldn't matter. It only seems to matter in some edge cases. And in those cases, people using bundlers can simply set |
Cool. Do you think we should add it in the docs/readme? |
The problem is for older versions of React which don't use/need the scheduler. I suppose we could add a little bit of logic to target folks who are using a version of React that uses the scheduler but we can't access it. If you want to do that, please do! |
🎉 This PR is included in version 10.4.5 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Closes #736
What: When we couldn't require the
scheduler
package we will now fallback toisModernScheduleCallbackSupported = false
meaning we will usecallback => callback()
.Why: Following the conversation in #736 - We found this to be a proper solution for now in case the
Scheduler
wasn't loaded.I still think we need to discuss this issue further and think if we can be able to figure a different way to require the
Scheduler
at runtime.How: I've added an if statement in the catch block to see if
isModernScheduleCallbackSupported
istrue
, in that case I change it tofalse
and write an error to console.I think that we may need to separate the try catch to two different ones and handle each error for it's own. Update me what you think please :)
Also, within the error message we may want to link to the documentation stating how to fix this one.
Checklist:
docs site - We need to implement a documentation change for this one, please update me where you think it fits.