Skip to content

Commit

Permalink
fix(core): fail to clean up framework observers
Browse files Browse the repository at this point in the history
  • Loading branch information
HuiSF committed Aug 11, 2023
1 parent a1664b4 commit 233bd08
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 6 deletions.
45 changes: 42 additions & 3 deletions packages/core/__tests__/Platform-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,18 @@ import { detectFramework, clearCache } from '../src/Platform/detectFramework';
import * as detection from '../src/Platform/detection';

describe('Platform test', () => {
beforeEach(() => clearCache());
beforeAll(() => {
jest.useFakeTimers();
});

afterAll(() => {
jest.useRealTimers();
});

beforeEach(() => {
clearCache();
});

describe('isReactNative test', () => {
test('happy case', () => {
expect(Platform.isReactNative).toBe(false);
Expand Down Expand Up @@ -70,8 +81,6 @@ describe('Platform test', () => {

describe('detectFramework', () => {
test('retries detection after 10ms', () => {
jest.useFakeTimers();

jest.spyOn(detection, 'detect');

detectFramework();
Expand All @@ -81,3 +90,33 @@ describe('Platform test', () => {
});
});
});

describe('detectFramework observers', () => {
let module;

beforeAll(() => {
jest.resetModules();
module = require('../src/Platform/detectFramework');
jest.useFakeTimers();
});

afterAll(() => {
jest.useRealTimers();
});

test('it notifies and cleans up the observers and rejects new observer after detection completes', () => {
const mockObserver = jest.fn();
module.observeFrameworkChanges(mockObserver);
expect(module.frameworkChangeObservers.length).toBe(1);

module.detectFramework();
expect(mockObserver).toHaveBeenCalledTimes(1);
jest.runOnlyPendingTimers();
module.detectFramework();
expect(mockObserver).toHaveBeenCalledTimes(2);
expect(module.frameworkChangeObservers.length).toBe(0);

module.observeFrameworkChanges(mockObserver);
expect(module.frameworkChangeObservers.length).toBe(0);
});
});
23 changes: 20 additions & 3 deletions packages/core/src/Platform/detectFramework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { detect } from './detection';
// We want to cache detection since the framework won't change
let frameworkCache: Framework | undefined;

const frameworkChangeObservers: (() => void)[] = [];
export const frameworkChangeObservers: (() => void)[] = [];

// Setup the detection reset tracking / timeout delays
let resetTriggered = false;
Expand All @@ -19,8 +19,19 @@ export const detectFramework = (): Framework => {
if (!frameworkCache) {
frameworkCache = detect();

// Everytime we update the cache, call each observer function
frameworkChangeObservers.forEach(fcn => fcn());
if (resetTriggered) {
// The final run of detectFramework:
// Starting from this point, the `frameworkCache` becomes "final".
// So we don't need to notify the observers again so the observer
// can be removed after the final notice.
while (frameworkChangeObservers.length) {
frameworkChangeObservers.pop()();
}
} else {
// The first run of detectFramework:
// Every time we update the cache, call each observer function
frameworkChangeObservers.forEach(fcn => fcn());
}

// Retry once for either Unknown type after a delay (explained below)
resetTimeout(Framework.ServerSideUnknown, SSR_RESET_TIMEOUT);
Expand All @@ -33,6 +44,12 @@ export const detectFramework = (): Framework => {
* @internal Setup observer callback that will be called everytime the framework changes
*/
export const observeFrameworkChanges = (fcn: () => void) => {
// When the `frameworkCache` won't be updated again, we ignore all incoming
// observers.
if (resetTriggered) {
return;
}

frameworkChangeObservers.push(fcn);
};

Expand Down

0 comments on commit 233bd08

Please sign in to comment.