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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ios][android][sdk][ncl] background location, geofencing and background fetch #2338

Merged
merged 79 commits into from Dec 13, 2018

Conversation

@tsapeta
Copy link
Member

commented Oct 2, 2018

Why

Canny issues:

Related PR:

How

The core of this PR is to implement TaskManager that provides an API to manage tasks, especially these that run while the app is in the background. I hope I documented this piece enough so I don't have to write it here again 馃槃

This PR also adds three ways of how we can use background tasks:

  • Background location updates
    Location.startLocationUpdatesAsync, Location.stopLocationUpdatesAsync, Location.hasStartedLocationUpdatesAsync
  • Geofencing
    Location.startGeofencingAsync, Location.stopGeofencingAsync, Location.hasStartedGeofencingAsync
  • Background fetch
    expo-background-fetch module, iOS only so far

Also, to improve the quality of expo-location, I've deprecated enableHighAccuracy option and introduced a new one called accuracy. It allows to use all possible accuracies provided by iOS platform. Android has fewer options of accuracy, but I've made simple mapping (see https://github.com/expo/expo/pull/2338/files#diff-66bdbbc66d3410d9198aa17584b5409eR633).

All new modules and expo-location are now TypeScripted 馃帀

Test Plan

New APIs examples in native-component-list:

  • BackgroundFetch - shows the status and the date of the last background fetch which can be manually triggered in Xcode (Debug -> Simulate background fetch).
  • Location -> Background location map - it tracks your location by drawing a polyline.
  • Location -> Geofencing map - this one will work better once we merge #2316, as it should show local notifications when the device enters/exits a region.
  • TaskManager - shows all registered tasks for the experience.

There are also some quick tests in test-suite for the new methods in Location and TaskManager modules.

@tsapeta tsapeta added this to the M33 (SDK 31) milestone Oct 2, 2018

@tsapeta tsapeta force-pushed the @tsapeta/background-tasks branch from 67a6dde to 59417ec Oct 11, 2018

@tsapeta tsapeta force-pushed the @tsapeta/background-tasks branch from 59417ec to 7027492 Oct 12, 2018

@tsapeta

This comment has been minimized.

Copy link
Member Author

commented Oct 15, 2018

expo-background-fetch is a module that implements Background Fetch from iOS (it is also called Background App Refresh). See Apple docs for more details.
It can run your function periodically, when your app is in the background so you can refresh a data that you store in your app or do some other not long-running stuff. We're planning to make it available on Android as well, but not in the next release (maybe SDK 32?).

I can't promise geofencing will be a part of SDK31 but we're doing our best to make it happen 馃槃

@esamelson
Copy link
Member

left a comment

Whew, there is a lot of code here! 馃槄 I have a couple of small comments so far, but things are looking generally good. Excited to land this 馃帀 I haven't made it to expo-task-manager yet, so this is just a first pass.

One question -- how are we handling Always vs When In Use permissions on iOS?

Show resolved Hide resolved ...expoview/src/main/java/host/exp/exponent/headless/HeadlessAppLoader.java Outdated
Show resolved Hide resolved ios/Exponent/Kernel/HeadlessApp/EXHeadlessAppRecord.m Outdated
{
NSTimeInterval timeInterval = [minimumInterval doubleValue];
[[UIApplication sharedApplication] setMinimumBackgroundFetchInterval:timeInterval];
resolve([NSNull null]);

This comment has been minimized.

Copy link
@esamelson

esamelson Oct 16, 2018

Member

curious why you're resolving all of these promises with explicit null instead of just nil (which I believe maps to undefined in JS)?

This comment has been minimized.

Copy link
@tsapeta

tsapeta Nov 14, 2018

Author Member

Since we can't resolve promises to undefined on Android, I think it would be a good idea to always return null to JavaScript, so it's always clear how to type the return value in TypeScript (Promise<null>). Also, it might be confusing that it returns undefined on iOS and null on Android 馃槄 So I just wanted to make it consistent.

This comment has been minimized.

Copy link
@esamelson

esamelson Dec 4, 2018

Member

IMO in those cases where no value is ever passed back, we should just not return the awaited promise in the TS code. I think it's fairly standard for promises to resolve with undefined in JS -- e.g. that's what happens if you just call .resolve().

Show resolved Hide resolved ...ages/expo-location/ios/EXLocation/TaskConsumers/EXLocationTaskConsumer.m Outdated
@ide
Copy link
Member

left a comment

There's a lot to review here and it will take awhile. I haven't looked at the module implementations yet. The TypeScript support needs to be consolidated with a central configuration so that all of the modules are built in a consistent way -- we are starting to work on this.

Show resolved Hide resolved ...oview/src/main/java/host/exp/exponent/experience/ExperienceActivity.java Outdated
Show resolved Hide resolved packages/expo/src/Expo.ts
@@ -0,0 +1,20 @@
{

This comment has been minimized.

Copy link
@ide

ide Oct 17, 2018

Member

Managing dozens of tsconfig.json files is going to be very difficult to keep in sync. Make this:

{
  "extends": "../expo/tsconfig"
}

This comment has been minimized.

Copy link
@tsapeta

tsapeta Nov 14, 2018

Author Member

Actually, I don't think this will work seamlessly - we would need to change rootDir of the expo/tsconfig.json file to be .. instead of ./src and then universal modules will be compiled into packages/expo/build folder and I guess it's not what we're going to achieve. Can I keep these configs for now until we find a good solution?

Show resolved Hide resolved packages/expo-location/package.json Outdated
Show resolved Hide resolved packages/expo-location/package.json Outdated
Show resolved Hide resolved apps/native-component-list/package.json Outdated
Show resolved Hide resolved ..._0_0/EXPermissions/ABI30_0_0EXPermissions/ABI30_0_0EXLocationRequester.m Outdated

@tsapeta tsapeta force-pushed the @tsapeta/background-tasks branch from 7027492 to c750f7e Oct 24, 2018

@tsapeta tsapeta modified the milestones: M33 (SDK 31), M34 Oct 24, 2018

@tsapeta tsapeta force-pushed the @tsapeta/background-tasks branch from c750f7e to c111160 Nov 7, 2018

@tsapeta tsapeta force-pushed the @tsapeta/background-tasks branch 5 times, most recently from 023f2b7 to 7e2d249 Nov 13, 2018

@tsapeta tsapeta force-pushed the @tsapeta/background-tasks branch from 7e2d249 to 6de1130 Nov 13, 2018

tsapeta added some commits Nov 14, 2018

@tsapeta tsapeta changed the title [WIP][ios][sdk] background location, geofencing and background fetch [ios][android][sdk][ncl] background location, geofencing and background fetch Nov 14, 2018

tsapeta added some commits Dec 12, 2018

@ide

ide approved these changes Dec 12, 2018

tsapeta added some commits Dec 13, 2018

@sjchmiela sjchmiela merged commit 3b6a24f into master Dec 13, 2018

2 checks passed

client Workflow: client
Details
sdk Workflow: sdk
Details

@sjchmiela sjchmiela deleted the @tsapeta/background-tasks branch Dec 13, 2018

@sjchmiela sjchmiela removed the in progress label Dec 13, 2018

@dgreene3p

This comment has been minimized.

Copy link

commented Dec 13, 2018

I cannot cheer enough for this merge!!!!! Thank you all so much for all the hard work you've done on this!!!!!

@dereknelson

This comment has been minimized.

Copy link
Contributor

commented Dec 13, 2018

I subscribed to email notifications for this thread and would still check this PR a few times a day. This is huge for my app, couldn't be more excited! Major thanks to everyone involved!!

@crudexec

This comment has been minimized.

Copy link

commented Dec 14, 2018

This is awesome, @tsapeta is background audio a possibility after this PR gets merged in?

@ardyfeb

This comment has been minimized.

Copy link

commented Dec 14, 2018

@tsapeta its possible add background-fetch for android, and nofitication scheduling on background ?

@tsapeta

This comment has been minimized.

Copy link
Member Author

commented Dec 14, 2018

@crudexec
Background audio is a little bit different thing because your app cannot be terminated by the system if the app plays something, so the system also doesn't wake up the app because it's still alive. In other words, there is no need to initiate JS context in the background. Anyway, we're planning such feature but right now I can't estimate when it will be done.

@ardyfeb
Actually there is no something like background fetch on Android, but of course we can achieve similar functionality using JobScheduler or AlarmManager APIs. We will implement this thing someday, but I also can't tell any estimation.

I also wanted to post a link to the docs associated with this pull request (as the link provided in the description is to the private repo): expo/expo-docs@9ef7842
I'll be glad to answer any questions you may have about this specification 馃槈

@ardyfeb

This comment has been minimized.

Copy link

commented Dec 14, 2018

@tsapeta i want to make chat app, where the app can listen socket on background and send notification to user. This case cannot use JobScheduler

@Slapbox

This comment has been minimized.

Copy link

commented Dec 14, 2018

@tsapeta thanks for taking the time to respond to questions and for your hard work!

Is any work underway on the features necessary to build an alarm clock app with Expo?

@esamelson

This comment has been minimized.

Copy link
Member

commented Dec 14, 2018

Hi all -- we appreciate the enthusiasm for this PR, but this thread is not the place for feature requests. Please use canny for that instead. Thanks 馃檪

@Slapbox

This comment has been minimized.

Copy link

commented Dec 14, 2018

@esamelson you're definitely right that this isn't the place for feature requests. The issue though, is that there's one big issue for all the things people are asking about and there's not really any updates there as far as what exactly is being worked on. This is mentioned as the relevant PR for that feature request. As awesome as this PR is, it only covers a subset of background tasks.

Rather than trying to field all the questions here, can anyone give the many watchers of this issue an idea of how future/current plans might be communicated to the community?

The real issue causing questions here is that although the request on Canny is marked "In Progress" it's not entirely clear what sort of background task support is actually In Progress or even planned down the road. Can we expect a more general background task capability in the future?

@esamelson

This comment has been minimized.

Copy link
Member

commented Dec 15, 2018

@Slapbox -- thanks for the clarification. "Background tasks support" can mean completely different things in terms of the implementation, depending on the use case. As of writing this, we have no concrete plans to add background task capabilities beyond this PR to the Expo SDK.

To answer your question about current & future plans, all our client-side work is now in this open-source repo, so watching the open PRs is the best way to understand our short-term plans / current work. Regarding longer-term plans, we try to communicate those on our blog when appropriate.

We are shifting our focus away from developing new modules and towards improving the ExpoKit experience and our infrastructure so that we can better empower developers to add features they need themselves. The maintenance cost for our existing modules is already quite high; further, it's virtually impossible to cover 100% of use cases with any limited set of native modules, and it's unsustainable to have our small team as a feature bottleneck.

So at this time, I would say no, you should not expect a more general background task capability in a future Expo SDK; however, you can expect an improved ExpoKit and infrastructure to which you can more easily add whatever capabilities you need, if they are not fulfilled by this PR or other Expo SDK modules.

I hope this makes sense and clarifies things a bit! 馃檪

@Slapbox

This comment has been minimized.

Copy link

commented Dec 15, 2018

Thanks a ton @esamelson, that's a tremendous help!

@morajabi

This comment has been minimized.

Copy link

commented Dec 15, 2018

Congrats and well-done everyone, millions of thanks! <3 I'm a tiny bit confused, is there a way to use this right now?

@Slapbox

This comment has been minimized.

Copy link

commented Dec 15, 2018

@morajabi I assume this is going to be included in Expo 32, which will probably be out any day now judging by the release history.

@mapet241

This comment has been minimized.

Copy link

commented Dec 18, 2018

Hi guys,
So is it already known if this feature is going to be a part of release 32? And when is release 32 planned to happen?

Many thanks for any news on that

@tsapeta

This comment has been minimized.

Copy link
Member Author

commented Dec 18, 2018

Hey @mapet241,
Yes, this feature will be a part of SDK32. We're now working on shipping it, you can track the release here: #2969

@mapet241

This comment has been minimized.

Copy link

commented Dec 18, 2018

Hey @mapet241,
Yes, this feature will be a part of SDK32. We're now working on shipping it, you can track the release here: #2969

Sounds great. From my perspective that is actually the only major item missing from expo that was preventing it's use in some important business scenarios. Thank you for covering that!

@wishfulthinkerme

This comment has been minimized.

Copy link
Contributor

commented Dec 18, 2018

Hey @mapet241,
Yes, this feature will be a part of SDK32. We're now working on shipping it, you can track the release here: #2969

Sounds great. From my perspective that is actually the only major item missing from expo that was preventing it's use in some important business scenarios. Thank you for covering that!

Also, NFC is missing. After that, I'll be the happiest dev in the world! Good job guys!

@sjchmiela

This comment has been minimized.

Copy link
Contributor

commented Dec 18, 2018

I'll let myself lock this conversation as this pull request has been merged and is already undergoing QA and awaiting release. I think this whole thread, thanks to @tsapeta's and @esamelson's explanations, will still be a valid source of information about this feature. 馃檪

For casual questions, please head over to Expo's Slack (https://slack.expo.io/).

We're really excited about this and all other features too!

When it comes to NFC support, as all feature requests it lives at expo.canny.io (https://expo.canny.io/feature-requests/p/nfc-support) and this will be the best source of information whether it's planned, in progress or resolved. 馃檪

@expo expo locked as resolved and limited conversation to collaborators Dec 18, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can鈥檛 perform that action at this time.