Quick fix for grabbing wrong rAF polyfill in ReactScheduler#12831
Closed
flarnie wants to merge 2 commits intofacebook:masterfrom
Closed
Quick fix for grabbing wrong rAF polyfill in ReactScheduler#12831flarnie wants to merge 2 commits intofacebook:masterfrom
flarnie wants to merge 2 commits intofacebook:masterfrom
Conversation
**what is the change?:** We need to grab a slightly different implementation of rAF internally at FB than in Open Source. Making rAF a dependency of the ReactScheduler module allows us to fork the dependency at FB. **why make this change?:** This fixes a problem we're running into when experimenting with ReactScheduler internally at Facebook, **and* it's part of our long term plan to use dependency injection with the scheduler to make it easier to test and adjust. **test plan:** Ran tests, lint, flow, and will manually test when syncing into Facebook's codebase. **issue:** See internal task T29442940
**what is the change?:** see title **why make this change?:** I realized we don't want Flow to run on this file. **test plan:** yarn flow
Contributor
wut? I don't think we should use DI. We should just fix FB to properly allow any third party code (like React) to use the native rAF. |
Contributor
Author
|
Maybe I should message you @sebmarkbage - this was something Andrew and I discussed. But for a short term fix we can also change things on the Facebook side. |
Contributor
|
We've worked really hard to avoid forks so not happy seeing them return. (We're almost ready to get rid of the forked event emitters.) |
gaearon
approved these changes
May 16, 2018
Collaborator
|
(Didn't see Seb's comment before approving. I agree we should probably fix it in www, we know how hard is it?) |
Contributor
Author
|
I'll investigate fixing it on www, closing this for now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what is the change?:
We need to grab a slightly different implementation of rAF internally at
FB than in Open Source. Making rAF a dependency of the ReactScheduler
module allows us to fork the dependency at FB.
why make this change?:
This fixes a problem we're running into when experimenting with
ReactScheduler internally at Facebook, *and it's part of our long term
plan to use dependency injection with the scheduler to make it easier to
test and adjust.
test plan:
Ran tests, lint, flow, and will manually test when syncing into
Facebook's codebase.
issue:
See internal task T29442940