-
Notifications
You must be signed in to change notification settings - Fork 47.3k
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
Fixed scheduler setTimeout fallback #14358
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# `jest-mock-scheduler` | ||
|
||
Jest matchers and utilities for testing the `scheduler` package. | ||
|
||
This package is experimental. APIs may change between releases. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
/** | ||
* Copyright (c) Facebook, Inc. and its affiliates. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
export * from './src/JestMockScheduler'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
'use strict'; | ||
|
||
if (process.env.NODE_ENV === 'production') { | ||
module.exports = require('./cjs/jest-mock-scheduler.production.min.js'); | ||
} else { | ||
module.exports = require('./cjs/jest-mock-scheduler.development.js'); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
{ | ||
"name": "jest-mock-scheduler", | ||
"private": true, | ||
"version": "0.1.0", | ||
"description": "Jest matchers and utilities for testing the scheduler package.", | ||
"main": "index.js", | ||
"repository": "facebook/react", | ||
"keywords": [ | ||
"jest", | ||
"scheduler" | ||
], | ||
"license": "MIT", | ||
"bugs": { | ||
"url": "https://github.com/facebook/react/issues" | ||
}, | ||
"homepage": "https://reactjs.org/", | ||
"peerDependencies": { | ||
"jest": "^23.0.1", | ||
"scheduler": "^0.11.0" | ||
}, | ||
"files": [ | ||
"LICENSE", | ||
"README.md", | ||
"build-info.json", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's in it? Sorry if I missed it somewhere. Do we include it in all packages? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just some metadata about this build, e.g. It's useful particularly for the |
||
"index.js", | ||
"cjs/" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
/** | ||
* Copyright (c) Facebook, Inc. and its affiliates. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
// Max 31 bit integer. The max integer size in V8 for 32-bit systems. | ||
// Math.pow(2, 30) - 1 | ||
// 0b111111111111111111111111111111 | ||
const maxSigned31BitInt = 1073741823; | ||
|
||
export function mockRestore() { | ||
delete global._schedMock; | ||
} | ||
|
||
let callback = null; | ||
let currentTime = -1; | ||
|
||
function flushCallback(didTimeout, ms) { | ||
if (callback !== null) { | ||
let cb = callback; | ||
callback = null; | ||
try { | ||
currentTime = ms; | ||
cb(didTimeout); | ||
} finally { | ||
currentTime = -1; | ||
} | ||
} | ||
} | ||
|
||
function requestHostCallback(cb, ms) { | ||
if (currentTime !== -1) { | ||
// Protect against re-entrancy. | ||
setTimeout(requestHostCallback, 0, cb, ms); | ||
} else { | ||
callback = cb; | ||
setTimeout(flushCallback, ms, true, ms); | ||
setTimeout(flushCallback, maxSigned31BitInt, false, maxSigned31BitInt); | ||
} | ||
} | ||
|
||
function cancelHostCallback() { | ||
callback = null; | ||
} | ||
|
||
function shouldYieldToHost() { | ||
return false; | ||
} | ||
|
||
function getCurrentTime() { | ||
return currentTime === -1 ? 0 : currentTime; | ||
} | ||
|
||
global._schedMock = [ | ||
requestHostCallback, | ||
cancelHostCallback, | ||
shouldYieldToHost, | ||
getCurrentTime, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -447,41 +447,44 @@ var requestHostCallback; | |
var cancelHostCallback; | ||
var shouldYieldToHost; | ||
|
||
if (typeof window !== 'undefined' && window._schedMock) { | ||
var globalValue = null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't you mean |
||
if (typeof window !== 'undefined') { | ||
globalValue = window; | ||
} else if (typeof global !== 'undefined') { | ||
globalValue = global; | ||
} | ||
|
||
if (globalValue && globalValue._schedMock) { | ||
// Dynamic injection, only for testing purposes. | ||
var impl = window._schedMock; | ||
requestHostCallback = impl[0]; | ||
cancelHostCallback = impl[1]; | ||
shouldYieldToHost = impl[2]; | ||
var globalImpl = globalValue._schedMock; | ||
requestHostCallback = globalImpl[0]; | ||
cancelHostCallback = globalImpl[1]; | ||
shouldYieldToHost = globalImpl[2]; | ||
getCurrentTime = globalImpl[3]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a little awkward. I could combine these branches? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's combine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, combined in db14ad8 |
||
} else if ( | ||
// If Scheduler runs in a non-DOM environment, it falls back to a naive | ||
// implementation using setTimeout. | ||
typeof window === 'undefined' || | ||
// Check if MessageChannel is supported, too. | ||
typeof MessageChannel !== 'function' | ||
) { | ||
// If this accidentally gets imported in a non-browser environment, e.g. JavaScriptCore, | ||
// fallback to a naive implementation. | ||
var _callback = null; | ||
var _currentTime = -1; | ||
var _flushCallback = function(didTimeout, ms) { | ||
var _flushCallback = function(didTimeout) { | ||
if (_callback !== null) { | ||
var cb = _callback; | ||
_callback = null; | ||
try { | ||
_currentTime = ms; | ||
cb(didTimeout); | ||
} finally { | ||
_currentTime = -1; | ||
} | ||
cb(didTimeout); | ||
} | ||
}; | ||
requestHostCallback = function(cb, ms) { | ||
if (_currentTime !== -1) { | ||
if (_callback !== null) { | ||
// Protect against re-entrancy. | ||
setTimeout(requestHostCallback, 0, cb, ms); | ||
setTimeout(requestHostCallback, 0, cb); | ||
} else { | ||
_callback = cb; | ||
setTimeout(_flushCallback, ms, true, ms); | ||
setTimeout(_flushCallback, maxSigned31BitInt, false, maxSigned31BitInt); | ||
setTimeout(_flushCallback, 0, false); | ||
} | ||
}; | ||
cancelHostCallback = function() { | ||
|
@@ -490,9 +493,6 @@ if (typeof window !== 'undefined' && window._schedMock) { | |
shouldYieldToHost = function() { | ||
return false; | ||
}; | ||
getCurrentTime = function() { | ||
return _currentTime === -1 ? 0 : _currentTime; | ||
}; | ||
} else { | ||
if (typeof console !== 'undefined') { | ||
// TODO: Remove fb.me link | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,9 @@ describe('Scheduler', () => { | |
jest.useFakeTimers(); | ||
jest.resetModules(); | ||
|
||
const JestMockScheduler = require('jest-mock-scheduler'); | ||
JestMockScheduler.mockRestore(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This convention seemed fairly inline with existing Jest mock packages. |
||
|
||
let _flushWork = null; | ||
let isFlushing = false; | ||
let timeoutID = -1; | ||
|
@@ -123,16 +126,21 @@ describe('Scheduler', () => { | |
function shouldYieldToHost() { | ||
return endOfFrame <= currentTime; | ||
} | ||
function getCurrentTime() { | ||
return currentTime; | ||
} | ||
|
||
// Override host implementation | ||
delete global.performance; | ||
global.Date.now = () => { | ||
return currentTime; | ||
}; | ||
|
||
window._schedMock = [ | ||
requestHostCallback, | ||
cancelHostCallback, | ||
shouldYieldToHost, | ||
getCurrentTime, | ||
]; | ||
|
||
const Schedule = require('scheduler'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -396,6 +396,15 @@ const bundles = [ | |
externals: ['jest-diff'], | ||
}, | ||
|
||
/******* Jest Scheduler (experimental) *******/ | ||
{ | ||
bundleTypes: [NODE_DEV, NODE_PROD, FB_WWW_DEV, FB_WWW_PROD], | ||
moduleType: ISOMORPHIC, | ||
entry: 'jest-mock-scheduler', | ||
global: 'JestMockScheduler', | ||
externals: ['jest-diff'], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copypasta There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assumed this external was required for some reason, since it was listed as a dependency for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably copypasta too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh well. We can rip it out in of both then. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
}, | ||
|
||
/******* ESLint Plugin for Hooks (proposal) *******/ | ||
{ | ||
// TODO: it's awkward to create a bundle for this | ||
|
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.
Let's make this private for now until we're sure we want it?
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.
Sure!