Skip to content
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

feat: Add ConnectionListener and a default browser implementation #305

Merged
merged 1 commit into from Mar 29, 2020

Conversation

imownbey
Copy link
Contributor

@imownbey imownbey commented Mar 24, 2020

Motivation

online & offline support was disabled in React Native.

Solution

Added a ConnectionListener which is now used by PollingSubscriptions that defaults to a browser implementation. Default implementation should be safe to run in non-browser environments, defaulting to previous behavior of just always saying things are online.

Open questions

@ntucker
Copy link
Collaborator

ntucker commented Mar 25, 2020

Thanks for this great PR!

Unfortunately, branches not on this particular codebase don't get coverage automatically run. Can you run yarn test:coverage and paste results?

@imownbey
Copy link
Contributor Author

imownbey commented Mar 25, 2020

-------------------------------------------|---------|----------|---------|---------|-------------------

File % Stmts % Branch % Funcs % Lines Uncovered Line #s
All files 99.58 97.64 98.52 99.56
legacy/src 100 100 100 100
index.ts 100 100 100 100
normalizr/src 100 100 100 100
denormalize.js 100 100 100 100
index.js 0 0 0 0
normalize.js 100 100 100 100
normalizr/src/schemas 100 99.12 100 100
Array.js 100 100 100 100
Entity.js 100 96.77 100 100 4
ImmutableUtils.js 100 100 100 100
Object.js 100 100 100 100
Polymorphic.js 100 100 100 100
Union.js 100 100 100 100
Values.js 100 100 100 100
rest-hooks/src 100 83.33 100 100
actionTypes.ts 100 100 100 100
errors.ts 0 0 0 0
index.ts 100 83.33 100 100 52
rest-hooks/src/react-integration 100 91.67 100 100
NetworkErrorBoundary.tsx 100 100 100 100
context.ts 100 100 100 100
index.ts 100 83.33 100 100 5
rest-hooks/src/react-integration/hooks 100 100 100 100
hasUsableData.ts 100 100 100 100
index.ts 100 100 100 100
useCache.ts 100 100 100 100
useError.ts 100 100 100 100
useFetcher.ts 100 100 100 100
useInvalidator.ts 100 100 100 100
useMeta.ts 100 100 100 100
useResetter.ts 100 100 100 100
useResource.ts 100 100 100 100
useRetrieve.ts 100 100 100 100
useSubscription.ts 100 100 100 100
rest-hooks/src/react-integration/provider 100 100 100 100
CacheProvider.tsx 100 100 100 100
ExternalCacheProvider.tsx 100 100 100 100
PromiseifyMiddleware.ts 100 100 100 100
index.ts 100 100 100 100
rest-hooks/src/resource 98.45 94.23 96.55 98.41
Entity.ts 92 83.33 77.78 92 19,89
Resource.ts 100 100 100 100
SimpleRecord.ts 100 100 100 100
SimpleResource.ts 100 100 100 100
index.ts 100 83.33 100 100 9
normal.ts 100 100 100 100
paramsToString.ts 100 100 100 100
types.ts 100 100 100 100
rest-hooks/src/state 99.07 94.39 96.23 99.02
BrowserConnectionListener.ts 80 100 80 80 9
NetworkManager.ts 100 95.45 100 100 129
PollingSubscription.ts 98.41 93.75 90.91 98.36 150
RIC.ts 100 100 100 100
SubscriptionManager.ts 100 87.5 100 100 79,104
actionCreators.ts 100 100 100 100
applyUpdatersToResults.ts 100 100 100 100
reducer.ts 100 95.24 100 100 34
rest-hooks/src/state/merge 100 100 100 100
isMergeable.ts 100 100 100 100
mergeDeepCopy.ts 100 100 100 100
rest-hooks/src/state/selectors 100 100 100 100
buildInferredResults.ts 100 100 100 100
getEntityPath.ts 100 100 100 100
index.ts 100 100 100 100
useDenormalized.ts 100 100 100 100
use-enhanced-reducer/src 100 100 100 100
index.ts 100 100 100 100
useEnhancedReducer.ts 100 100 100 100
usePromisifiedDispatch.ts 100 100 100 100
------------------------------------------- --------- ---------- --------- --------- -------------------

Test Suites: 28 passed, 28 total
Tests: 37 skipped, 342 passed, 379 total
Snapshots: 195 passed, 195 total
Time: 15.708s
Ran all test suites.
✨ Done in 21.85s.

@@ -32,8 +35,15 @@ export default class PollingSubscription implements Subscription {
this.url = url;
this.frequencyHistogram.set(this.frequency, 1);
this.dispatch = dispatch;
if (isOnline()) this.update();
this.run();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, doesn't this.run() still need to happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It happens now by calling this.onlineListener() if we are online on line 43

@ntucker ntucker merged commit f63f66c into reactive:master Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants