-
Notifications
You must be signed in to change notification settings - Fork 18
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: RN streamer connection in background and foreground. #360
Merged
yusinto
merged 43 commits into
main
from
yus/sc-228044/rn-sdk-should-disable-streaming-in-background
Feb 1, 2024
Merged
Changes from all commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
2c7ac96
feat: Implement common support for auto environment attributes.
yusinto 53d4358
feat: Implement common client side support for auto environment attri…
yusinto dc3f3af
chore: Fixed server sdk tests due to mock api changes for auto env.
yusinto e552def
feat: React-native support for auto-env attributes.
yusinto 2028f4c
chore: Fix broken common tests due to mocks api changes.
yusinto e8989f1
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto 23a11dc
Merge branch 'main' into yus/client-sdk-auto-env
yusinto 1156f25
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto adc1de1
fix: Remove sdk data as fallback for auto env. Strip falsy values and…
yusinto 04bb98f
fix: Respect customer provided ld_application and ld_device contexts.
yusinto 75bb880
fix: Add mandatory autoEnvAttributes argument to LDClient constructor.
yusinto 5243f3f
chore: Move AutoEnvAttributes enum to common.
yusinto 22b97bd
fix: Make all device.os properties optional.
yusinto d365184
fix: Added mandatory AutoEnvAttributes constructor arg.
yusinto b47a5e2
chore: Log warning if auto env attributes are not added because they …
yusinto d1f0310
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto ebc670a
chore: Remove unused import in platform mock.
yusinto b5ce097
chore: Fix duplicated test name.
yusinto ccb8419
fix: Implemented separate namespaces for anon and contexts. Fixed bug…
yusinto 125f82b
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto 4e7f1c1
fix: Hardcode @types/node version to avoid crypto typings but in v20.…
yusinto caa1f14
chore: Add comment to explain harcoding of @types.node.
yusinto 6ef3a87
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto 4375509
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto ef15c18
Merge branch 'yus/rn-sdk-auto-env' of github.com:launchdarkly/js-core…
yusinto 189d379
fix: Import AutoEnvAttributes from the rn sdk instead of client common.
yusinto 31643eb
fix: Add tsconfig jsx react setting.
yusinto 1788a21
chore: Remove hardcoded @types/node version.
yusinto f851823
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto d9dc2e5
chore: Deleted redundant react-native-sse files and folder.
yusinto f6ca89c
fix: Added rn sdk streaming jitter backoff.
yusinto 50c884c
chore: Added backoff jitter tests.
yusinto aaa7f94
chore: Use Math.min instead of logical operators for backoff.
yusinto 704d3f2
chore: Skeletal background foreground detection logic.
yusinto a2395c5
chore: Add debounce function to common utils.
yusinto 4a2dcf2
chore: Added EventSource getStatus. Added useAppState hook.
yusinto 75ca7f6
chore: Move isEventSourceClosed function to useAppState hook.
yusinto dff6153
chore: Remove explicit debounce delay and use default instead. Remove…
yusinto cd6e4d3
chore: Add app state tests.
yusinto bcaba56
chore: Improve debug message. Added comments to useAppState hook.
yusinto e92a1d5
chore: Improve debounce comments.
yusinto aa48d57
Merge branch 'main' into yus/sc-228044/rn-sdk-should-disable-streamin…
yusinto 8b12c9b
chore: Add missing export.
yusinto File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
114 changes: 114 additions & 0 deletions
114
packages/sdk/react-native/src/provider/useAppState.test.ts
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
import { renderHook } from '@testing-library/react'; | ||
import React, { useRef } from 'react'; | ||
import { AppState } from 'react-native'; | ||
|
||
import { AutoEnvAttributes, debounce } from '@launchdarkly/js-client-sdk-common'; | ||
import { logger } from '@launchdarkly/private-js-mocks'; | ||
|
||
import EventSource from '../fromExternal/react-native-sse'; | ||
import ReactNativeLDClient from '../ReactNativeLDClient'; | ||
import useAppState from './useAppState'; | ||
|
||
jest.mock('@launchdarkly/js-client-sdk-common', () => { | ||
const actual = jest.requireActual('@launchdarkly/js-client-sdk-common'); | ||
return { | ||
...actual, | ||
debounce: jest.fn(), | ||
}; | ||
}); | ||
|
||
describe('useAppState', () => { | ||
const eventSourceOpen = 1; | ||
const eventSourceClosed = 2; | ||
|
||
let appStateSpy: jest.SpyInstance; | ||
let ldc: ReactNativeLDClient; | ||
let mockEventSource: Partial<EventSource>; | ||
|
||
beforeEach(() => { | ||
(debounce as jest.Mock).mockImplementation((f) => f); | ||
appStateSpy = jest.spyOn(AppState, 'addEventListener').mockReturnValue({ remove: jest.fn() }); | ||
jest.spyOn(React, 'useRef').mockReturnValue({ | ||
current: 'active', | ||
}); | ||
|
||
ldc = new ReactNativeLDClient('mob-test-key', AutoEnvAttributes.Enabled, { logger }); | ||
|
||
mockEventSource = { | ||
getStatus: jest.fn(() => eventSourceOpen), | ||
OPEN: eventSourceOpen, | ||
CLOSED: eventSourceClosed, | ||
}; | ||
// @ts-ignore | ||
ldc.platform.requests = { eventSource: mockEventSource }; | ||
// @ts-ignore | ||
ldc.streamer = { start: jest.fn().mockName('start'), stop: jest.fn().mockName('stop') }; | ||
}); | ||
|
||
afterEach(() => { | ||
jest.resetAllMocks(); | ||
}); | ||
|
||
test('stops streamer in background', () => { | ||
renderHook(() => useAppState(ldc)); | ||
const onChange = appStateSpy.mock.calls[0][1]; | ||
|
||
onChange('background'); | ||
|
||
expect(ldc.streamer?.stop).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
test('starts streamer transitioning from background to active', () => { | ||
(useRef as jest.Mock).mockReturnValue({ current: 'background' }); | ||
(mockEventSource.getStatus as jest.Mock).mockReturnValue(eventSourceClosed); | ||
|
||
renderHook(() => useAppState(ldc)); | ||
const onChange = appStateSpy.mock.calls[0][1]; | ||
|
||
onChange('active'); | ||
|
||
expect(ldc.streamer?.start).toHaveBeenCalledTimes(1); | ||
expect(ldc.streamer?.stop).not.toHaveBeenCalled(); | ||
}); | ||
|
||
test('starts streamer transitioning from inactive to active', () => { | ||
(useRef as jest.Mock).mockReturnValue({ current: 'inactive' }); | ||
(mockEventSource.getStatus as jest.Mock).mockReturnValue(eventSourceClosed); | ||
|
||
renderHook(() => useAppState(ldc)); | ||
const onChange = appStateSpy.mock.calls[0][1]; | ||
|
||
onChange('active'); | ||
|
||
expect(ldc.streamer?.start).toHaveBeenCalledTimes(1); | ||
expect(ldc.streamer?.stop).not.toHaveBeenCalled(); | ||
}); | ||
|
||
test('does not start streamer in foreground because event source is already open', () => { | ||
(useRef as jest.Mock).mockReturnValue({ current: 'background' }); | ||
(mockEventSource.getStatus as jest.Mock).mockReturnValue(eventSourceOpen); | ||
|
||
renderHook(() => useAppState(ldc)); | ||
const onChange = appStateSpy.mock.calls[0][1]; | ||
|
||
onChange('active'); | ||
|
||
expect(ldc.streamer?.start).not.toHaveBeenCalled(); | ||
expect(ldc.streamer?.stop).not.toHaveBeenCalled(); | ||
expect(ldc.logger.debug).toHaveBeenCalledWith(expect.stringMatching(/already open/)); | ||
}); | ||
|
||
test('active state unchanged no action needed', () => { | ||
(useRef as jest.Mock).mockReturnValue({ current: 'active' }); | ||
(mockEventSource.getStatus as jest.Mock).mockReturnValue(eventSourceClosed); | ||
|
||
renderHook(() => useAppState(ldc)); | ||
const onChange = appStateSpy.mock.calls[0][1]; | ||
|
||
onChange('active'); | ||
|
||
expect(ldc.streamer?.start).not.toHaveBeenCalled(); | ||
expect(ldc.streamer?.stop).not.toHaveBeenCalled(); | ||
expect(ldc.logger.debug).toHaveBeenCalledWith(expect.stringMatching(/no action needed/i)); | ||
}); | ||
}); |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import { useEffect, useRef } from 'react'; | ||
import { AppState, AppStateStatus } from 'react-native'; | ||
|
||
import { debounce } from '@launchdarkly/js-client-sdk-common'; | ||
|
||
import { PlatformRequests } from '../platform'; | ||
import ReactNativeLDClient from '../ReactNativeLDClient'; | ||
|
||
/** | ||
* Manages streamer connection based on AppState. Debouncing is used to prevent excessive starting | ||
* and stopping of the EventSource which are expensive. | ||
* | ||
* background to active - start streamer. | ||
* active to background - stop streamer. | ||
* | ||
* @param client | ||
*/ | ||
const useAppState = (client: ReactNativeLDClient) => { | ||
const appState = useRef(AppState.currentState); | ||
|
||
const isEventSourceClosed = () => { | ||
const { eventSource } = client.platform.requests as PlatformRequests; | ||
return eventSource?.getStatus() === eventSource?.CLOSED; | ||
}; | ||
|
||
const onChange = (nextAppState: AppStateStatus) => { | ||
client.logger.debug(`App state prev: ${appState.current}, next: ${nextAppState}`); | ||
|
||
if (appState.current.match(/inactive|background/) && nextAppState === 'active') { | ||
if (isEventSourceClosed()) { | ||
client.logger.debug('Starting streamer after transitioning to foreground.'); | ||
client.streamer?.start(); | ||
} else { | ||
client.logger.debug('Not starting streamer because EventSource is already open.'); | ||
} | ||
} else if (nextAppState === 'background') { | ||
client.logger.debug('App state background stopping streamer.'); | ||
client.streamer?.stop(); | ||
} else { | ||
client.logger.debug('No action needed.'); | ||
} | ||
|
||
appState.current = nextAppState; | ||
}; | ||
|
||
// debounce with a default delay of 5 seconds. | ||
const debouncedOnChange = debounce(onChange); | ||
|
||
useEffect(() => { | ||
const sub = AppState.addEventListener('change', debouncedOnChange); | ||
|
||
return () => { | ||
sub.remove(); | ||
}; | ||
}, []); | ||
}; | ||
|
||
export default useAppState; |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/** | ||
* Wait before calling the same function. Useful for expensive calls. | ||
* Adapted from https://amitd.co/code/typescript/debounce. | ||
* | ||
* @return The debounced function. | ||
* | ||
* @example | ||
* | ||
* ```js | ||
* const debouncedFunction = debounce(e => { | ||
* console.log(e); | ||
* }, 5000); | ||
* | ||
* // Console logs 'Hello world again ' after 5 seconds | ||
* debouncedFunction('Hello world'); | ||
* debouncedFunction('Hello world again'); | ||
* ``` | ||
* @param fn The function to be debounced. | ||
* @param delayMs Defaults to 5 seconds. | ||
*/ | ||
const debounce = <T extends (...args: any[]) => ReturnType<T>>( | ||
fn: T, | ||
delayMs: number = 5000, | ||
): ((...args: Parameters<T>) => void) => { | ||
let timer: ReturnType<typeof setTimeout>; | ||
|
||
return (...args: Parameters<T>) => { | ||
clearTimeout(timer); | ||
timer = setTimeout(() => { | ||
fn(...args); | ||
}, delayMs); | ||
}; | ||
}; | ||
|
||
export default debounce; |
This file contains 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
Oops, something went wrong.
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.
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.
This will likely not run sometimes on iOS, or maybe at all, when transitioning to the background. But that is probably fine, because restoring entering the foreground will be more important. My only concern will be if we end up basically superseding the transition with the retry.
Theoretically, on iOS, you only are certain to be able to execute during the transition, and not after. Which is the reason flutter doesn't debounce it, even though I would generally like to.