From 3c2a934602688ed2446c1536232a6ccb396d3ccc Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 4 Sep 2018 17:21:18 -0700 Subject: [PATCH] Cleaned up 'schedule' API wrt interactions and subscriber ref: * Removed 'private' ref methods from UMD forwarding API * Replaced getters with exported constants since they were no longer referenced for UMD forwarding --- fixtures/tracking/script.js | 4 +- .../src/ReactFiberScheduler.js | 41 +++++++------------ packages/react/src/ReactSharedInternals.js | 8 ++-- .../npm/umd/schedule-tracking.development.js | 16 -------- .../umd/schedule-tracking.production.min.js | 16 -------- packages/schedule/src/Tracking.js | 10 +---- .../schedule/src/TrackingSubscriptions.js | 10 ++--- .../ScheduleUMDBundle-test.internal.js | 14 ++++++- .../src/__tests__/Tracking-test.internal.js | 8 ++-- .../TrackingSubscriptions-test.internal.js | 10 ++--- packages/shared/forks/ScheduleTracking.umd.js | 8 ++-- 11 files changed, 50 insertions(+), 95 deletions(-) diff --git a/fixtures/tracking/script.js b/fixtures/tracking/script.js index 80d453a86bdbe..c1ba29b951959 100644 --- a/fixtures/tracking/script.js +++ b/fixtures/tracking/script.js @@ -48,8 +48,6 @@ function checkSchedulerTrackingAPI() { runTest(document.getElementById('checkSchedulerTrackingAPI'), () => { if ( typeof ScheduleTracking === 'undefined' || - typeof ScheduleTracking.__getInteractionsRef !== 'function' || - typeof ScheduleTracking.__getSubscriberRef !== 'function' || typeof ScheduleTracking.unstable_clear !== 'function' || typeof ScheduleTracking.unstable_getCurrent !== 'function' || typeof ScheduleTracking.unstable_getThreadID !== 'function' || @@ -62,7 +60,7 @@ function checkSchedulerTrackingAPI() { try { let interactionsSet; ScheduleTracking.unstable_track('test', 123, () => { - interactionsSet = ScheduleTracking.__getInteractionsRef().current; + interactionsSet = ScheduleTracking.unstable_getCurrent(); }); if (interactionsSet.size !== 1) { throw null; diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index c5eb421a0616e..327cafccbc1fc 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -10,13 +10,9 @@ import type {Fiber} from './ReactFiber'; import type {Batch, FiberRoot} from './ReactFiberRoot'; import type {ExpirationTime} from './ReactFiberExpirationTime'; -import type { - Interaction, - InteractionsRef, - SubscriberRef, -} from 'schedule/src/Tracking'; +import type {Interaction} from 'schedule/src/Tracking'; -import {__getInteractionsRef, __getSubscriberRef} from 'schedule/tracking'; +import {__interactionsRef, __subscriberRef} from 'schedule/tracking'; import { invokeGuardedCallback, hasCaughtError, @@ -166,13 +162,6 @@ export type Thenable = { const {ReactCurrentOwner} = ReactSharedInternals; -let interactionsRef: InteractionsRef = (null: any); -let subscriberRef: SubscriberRef = (null: any); -if (enableSchedulerTracking) { - interactionsRef = __getInteractionsRef(); - subscriberRef = __getSubscriberRef(); -} - let didWarnAboutStateTransition; let didWarnSetStateChildContext; let warnAboutUpdateOnUnmounted; @@ -570,8 +559,8 @@ function commitRoot(root: FiberRoot, finishedWork: Fiber): void { if (enableSchedulerTracking) { // Restore any pending interactions at this point, // So that cascading work triggered during the render phase will be accounted for. - prevInteractions = interactionsRef.current; - interactionsRef.current = root.memoizedInteractions; + prevInteractions = __interactionsRef.current; + __interactionsRef.current = root.memoizedInteractions; // We are potentially finished with the current batch of interactions. // So we should clear them out of the pending interaction map. @@ -766,12 +755,12 @@ function commitRoot(root: FiberRoot, finishedWork: Fiber): void { onCommit(root, earliestRemainingTimeAfterCommit); if (enableSchedulerTracking) { - interactionsRef.current = prevInteractions; + __interactionsRef.current = prevInteractions; let subscriber; try { - subscriber = subscriberRef.current; + subscriber = __subscriberRef.current; if (subscriber !== null && root.memoizedInteractions.size > 0) { const threadID = computeThreadID( committedExpirationTime, @@ -1176,8 +1165,8 @@ function renderRoot( if (enableSchedulerTracking) { // We're about to start new tracked work. // Restore pending interactions so cascading work triggered during the render phase will be accounted for. - prevInteractions = interactionsRef.current; - interactionsRef.current = root.memoizedInteractions; + prevInteractions = __interactionsRef.current; + __interactionsRef.current = root.memoizedInteractions; } // Check if we're starting from a fresh stack, or if we're resuming from @@ -1220,7 +1209,7 @@ function renderRoot( root.memoizedInteractions = interactions; if (interactions.size > 0) { - const subscriber = subscriberRef.current; + const subscriber = __subscriberRef.current; if (subscriber !== null) { const threadID = computeThreadID( expirationTime, @@ -1305,7 +1294,7 @@ function renderRoot( if (enableSchedulerTracking) { // Tracked work is done for now; restore the previous interactions. - interactionsRef.current = prevInteractions; + __interactionsRef.current = prevInteractions; } // We're done performing work. Time to clean up. @@ -1614,13 +1603,13 @@ function retrySuspendedRoot( if (rootExpirationTime !== NoWork) { if (enableSchedulerTracking) { // Restore previous interactions so that new work is associated with them. - let prevInteractions = interactionsRef.current; - interactionsRef.current = root.memoizedInteractions; + let prevInteractions = __interactionsRef.current; + __interactionsRef.current = root.memoizedInteractions; // Because suspense timeouts do not decrement the interaction count, // Continued suspense work should also not increment the count. storeInteractionsForExpirationTime(root, rootExpirationTime, false); requestWork(root, rootExpirationTime); - interactionsRef.current = prevInteractions; + __interactionsRef.current = prevInteractions; } else { requestWork(root, rootExpirationTime); } @@ -1687,7 +1676,7 @@ function storeInteractionsForExpirationTime( return; } - const interactions = interactionsRef.current; + const interactions = __interactionsRef.current; if (interactions.size > 0) { const pendingInteractions = root.pendingInteractionMap.get(expirationTime); if (pendingInteractions != null) { @@ -1710,7 +1699,7 @@ function storeInteractionsForExpirationTime( } } - const subscriber = subscriberRef.current; + const subscriber = __subscriberRef.current; if (subscriber !== null) { const threadID = computeThreadID( expirationTime, diff --git a/packages/react/src/ReactSharedInternals.js b/packages/react/src/ReactSharedInternals.js index e79baa1b3a5bc..0247b96c758ee 100644 --- a/packages/react/src/ReactSharedInternals.js +++ b/packages/react/src/ReactSharedInternals.js @@ -12,8 +12,8 @@ import { unstable_scheduleWork, } from 'schedule'; import { - __getInteractionsRef, - __getSubscriberRef, + __interactionsRef, + __subscriberRef, unstable_clear, unstable_getCurrent, unstable_getThreadID, @@ -44,8 +44,8 @@ if (__UMD__) { unstable_scheduleWork, }, ScheduleTracking: { - __getInteractionsRef, - __getSubscriberRef, + __interactionsRef, + __subscriberRef, unstable_clear, unstable_getCurrent, unstable_getThreadID, diff --git a/packages/schedule/npm/umd/schedule-tracking.development.js b/packages/schedule/npm/umd/schedule-tracking.development.js index 73a89d6ab0154..00ce22d125aa7 100644 --- a/packages/schedule/npm/umd/schedule-tracking.development.js +++ b/packages/schedule/npm/umd/schedule-tracking.development.js @@ -16,20 +16,6 @@ ? define(['react'], factory) // eslint-disable-line no-undef : (global.ScheduleTracking = factory(global)); })(this, function(global) { - function __getInteractionsRef() { - return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ScheduleTracking.__getInteractionsRef.apply( - this, - arguments - ); - } - - function __getSubscriberRef() { - return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ScheduleTracking.__getSubscriberRef.apply( - this, - arguments - ); - } - function unstable_clear() { return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ScheduleTracking.unstable_clear.apply( this, @@ -82,8 +68,6 @@ } return Object.freeze({ - __getInteractionsRef: __getInteractionsRef, - __getSubscriberRef: __getSubscriberRef, unstable_clear: unstable_clear, unstable_getCurrent: unstable_getCurrent, unstable_getThreadID: unstable_getThreadID, diff --git a/packages/schedule/npm/umd/schedule-tracking.production.min.js b/packages/schedule/npm/umd/schedule-tracking.production.min.js index 73a89d6ab0154..00ce22d125aa7 100644 --- a/packages/schedule/npm/umd/schedule-tracking.production.min.js +++ b/packages/schedule/npm/umd/schedule-tracking.production.min.js @@ -16,20 +16,6 @@ ? define(['react'], factory) // eslint-disable-line no-undef : (global.ScheduleTracking = factory(global)); })(this, function(global) { - function __getInteractionsRef() { - return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ScheduleTracking.__getInteractionsRef.apply( - this, - arguments - ); - } - - function __getSubscriberRef() { - return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ScheduleTracking.__getSubscriberRef.apply( - this, - arguments - ); - } - function unstable_clear() { return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ScheduleTracking.unstable_clear.apply( this, @@ -82,8 +68,6 @@ } return Object.freeze({ - __getInteractionsRef: __getInteractionsRef, - __getSubscriberRef: __getSubscriberRef, unstable_clear: unstable_clear, unstable_getCurrent: unstable_getCurrent, unstable_getThreadID: unstable_getThreadID, diff --git a/packages/schedule/src/Tracking.js b/packages/schedule/src/Tracking.js index 6678ad3cc9a1d..7c69705a978d6 100644 --- a/packages/schedule/src/Tracking.js +++ b/packages/schedule/src/Tracking.js @@ -79,15 +79,7 @@ if (enableSchedulerTracking) { }; } -// These values are exported for libraries with advanced use cases (i.e. React). -// They should not typically be accessed directly. -export function __getInteractionsRef(): InteractionsRef { - return interactionsRef; -} - -export function __getSubscriberRef(): SubscriberRef { - return subscriberRef; -} +export {interactionsRef as __interactionsRef, subscriberRef as __subscriberRef}; export function unstable_clear(callback: Function): any { if (!enableSchedulerTracking) { diff --git a/packages/schedule/src/TrackingSubscriptions.js b/packages/schedule/src/TrackingSubscriptions.js index ee598dceec446..a847b28ccb6f4 100644 --- a/packages/schedule/src/TrackingSubscriptions.js +++ b/packages/schedule/src/TrackingSubscriptions.js @@ -7,15 +7,13 @@ * @flow */ -import type {Interaction, Subscriber, SubscriberRef} from './Tracking'; +import type {Interaction, Subscriber} from './Tracking'; import {enableSchedulerTracking} from 'shared/ReactFeatureFlags'; -import {__getSubscriberRef} from 'schedule/tracking'; +import {__subscriberRef} from 'schedule/tracking'; -let subscriberRef: SubscriberRef = (null: any); let subscribers: Set = (null: any); if (enableSchedulerTracking) { - subscriberRef = __getSubscriberRef(); subscribers = new Set(); } @@ -24,7 +22,7 @@ export function unstable_subscribe(subscriber: Subscriber): void { subscribers.add(subscriber); if (subscribers.size === 1) { - subscriberRef.current = { + __subscriberRef.current = { onInteractionScheduledWorkCompleted, onInteractionTracked, onWorkCanceled, @@ -41,7 +39,7 @@ export function unstable_unsubscribe(subscriber: Subscriber): void { subscribers.delete(subscriber); if (subscribers.size === 0) { - subscriberRef.current = null; + __subscriberRef.current = null; } } } diff --git a/packages/schedule/src/__tests__/ScheduleUMDBundle-test.internal.js b/packages/schedule/src/__tests__/ScheduleUMDBundle-test.internal.js index d21c8be891a18..3c1112c6eafa0 100644 --- a/packages/schedule/src/__tests__/ScheduleUMDBundle-test.internal.js +++ b/packages/schedule/src/__tests__/ScheduleUMDBundle-test.internal.js @@ -16,10 +16,20 @@ describe('Scheduling UMD bundle', () => { jest.resetModules(); }); + function filterPrivateKeys(name) { + return !name.startsWith('_'); + } + function validateForwardedAPIs(api, forwardedAPIs) { - const apiKeys = Object.keys(api).sort(); + const apiKeys = Object.keys(api) + .filter(filterPrivateKeys) + .sort(); forwardedAPIs.forEach(forwardedAPI => { - expect(Object.keys(forwardedAPI).sort()).toEqual(apiKeys); + expect( + Object.keys(forwardedAPI) + .filter(filterPrivateKeys) + .sort(), + ).toEqual(apiKeys); }); } diff --git a/packages/schedule/src/__tests__/Tracking-test.internal.js b/packages/schedule/src/__tests__/Tracking-test.internal.js index 948a415209d85..f4e56a366a50c 100644 --- a/packages/schedule/src/__tests__/Tracking-test.internal.js +++ b/packages/schedule/src/__tests__/Tracking-test.internal.js @@ -299,11 +299,11 @@ describe('Tracking', () => { it('should expose the current set of interactions to be externally manipulated', () => { SchedulerTracking.unstable_track('outer event', currentTime, () => { - expect(SchedulerTracking.__getInteractionsRef().current).toBe( + expect(SchedulerTracking.__interactionsRef.current).toBe( SchedulerTracking.unstable_getCurrent(), ); - SchedulerTracking.__getInteractionsRef().current = new Set([ + SchedulerTracking.__interactionsRef.current = new Set([ {name: 'override event'}, ]); @@ -315,7 +315,7 @@ describe('Tracking', () => { it('should expose a subscriber ref to be externally manipulated', () => { SchedulerTracking.unstable_track('outer event', currentTime, () => { - expect(SchedulerTracking.__getSubscriberRef()).toEqual({ + expect(SchedulerTracking.__subscriberRef).toEqual({ current: null, }); }); @@ -368,7 +368,7 @@ describe('Tracking', () => { describe('advanced integration', () => { it('should not create unnecessary objects', () => { - expect(SchedulerTracking.__getInteractionsRef()).toBe(null); + expect(SchedulerTracking.__interactionsRef).toBe(null); }); }); }); diff --git a/packages/schedule/src/__tests__/TrackingSubscriptions-test.internal.js b/packages/schedule/src/__tests__/TrackingSubscriptions-test.internal.js index a6209018ea571..6ecb9d75a44de 100644 --- a/packages/schedule/src/__tests__/TrackingSubscriptions-test.internal.js +++ b/packages/schedule/src/__tests__/TrackingSubscriptions-test.internal.js @@ -112,15 +112,15 @@ describe('TrackingSubscriptions', () => { it('should lazily subscribe to tracking and unsubscribe again if there are no external subscribers', () => { loadModules({enableSchedulerTracking: true, autoSubscribe: false}); - expect(SchedulerTracking.__getSubscriberRef().current).toBe(null); + expect(SchedulerTracking.__subscriberRef.current).toBe(null); SchedulerTracking.unstable_subscribe(firstSubscriber); - expect(SchedulerTracking.__getSubscriberRef().current).toBeDefined(); + expect(SchedulerTracking.__subscriberRef.current).toBeDefined(); SchedulerTracking.unstable_subscribe(secondSubscriber); - expect(SchedulerTracking.__getSubscriberRef().current).toBeDefined(); + expect(SchedulerTracking.__subscriberRef.current).toBeDefined(); SchedulerTracking.unstable_unsubscribe(secondSubscriber); - expect(SchedulerTracking.__getSubscriberRef().current).toBeDefined(); + expect(SchedulerTracking.__subscriberRef.current).toBeDefined(); SchedulerTracking.unstable_unsubscribe(firstSubscriber); - expect(SchedulerTracking.__getSubscriberRef().current).toBe(null); + expect(SchedulerTracking.__subscriberRef.current).toBe(null); }); describe('error handling', () => { diff --git a/packages/shared/forks/ScheduleTracking.umd.js b/packages/shared/forks/ScheduleTracking.umd.js index af31a4867464b..04a1fd82654ed 100644 --- a/packages/shared/forks/ScheduleTracking.umd.js +++ b/packages/shared/forks/ScheduleTracking.umd.js @@ -12,8 +12,8 @@ import React from 'react'; const ReactInternals = React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED; const { - __getInteractionsRef, - __getSubscriberRef, + __interactionsRef, + __subscriberRef, unstable_clear, unstable_getCurrent, unstable_getThreadID, @@ -24,8 +24,8 @@ const { } = ReactInternals.ScheduleTracking; export { - __getInteractionsRef, - __getSubscriberRef, + __interactionsRef, + __subscriberRef, unstable_clear, unstable_getCurrent, unstable_getThreadID,