[task-manager] correct the return type of TaskManagerTaskExecutor

[Simek]

Why

Should fix:

• [docs] BackgroundFetch example does not match the type #32414

How

Add missing Promise to the TaskManagerTaskExecutor return type. We were already awaiting the response:

• expo/packages/expo-task-manager/src/TaskManager.ts

  Line 264 in 6da2292

  result = await taskExecutor({ data, error, executionInfo });

Test Plan

The package lints correctly. The docs changes have been reviewed by running app locally.

Preview

[github-actions]

Subscribed to pull request

File Patterns	Mentions
docs/**	@amandeepmittal
packages/expo-task-manager/**	@tsapeta, @byCedric

Generated by CodeMention

[trajano]

rather than Promise<any> it may be better to have

export type TaskManagerTaskExecutor<T = unknown, R= unknown> = (body: TaskManagerTaskBody<T>) => Promise<R>;

[Simek]

I was thinking about introducing an another generic, but if you look at the code, we already were casting the result to any, and it would be a breaking change for all of the task manager users in the TS project, and since we are close to the stable SDK release I decided not to introduce breaking change that late in the process.

However, we might revisit it this in the future, and follow up with more strict typing.

[trajano]

OK but I don't think it should break as you added a "default". The default can be any as well to match your result.

FYI you already changed the default of the data type original from unknown to any.

[Simek]

I'm talking about the typing which we apply for the returned result in the handling process, it was any for few years already, and I have changed unknown to match that:

• https://github.com/expo/expo/blame/6da22921f35db183fc6722c1b697ff77d820387c/packages/expo-task-manager/src/TaskManager.ts#L259

[trajano]

Ok ...

I was wondering why T=unknown changed to T=any?

Also

adding a typing after so it's <T=unknown,R=unknown> would not break the type because Promise<R> which resolves to R would get matched with any

What it does allow you to do is for BackgroundFetch as an example in a future API

BackgroundFetch.defineTask(task:TaskManagerExecutor<Record<string,unknown>, BackgroundFetchResult>)

So it's a specific strongly typed version of defineTask for BackgroundFetch.

I chose Record<string,unknown> because the data varies from APNS vs FCM as I have noted in

• https://github.com/trajano/expo-experiments/blob/master/packages/my-app/src/notifications/extractDataFromBackgroundNotification.ts
• https://github.com/trajano/expo-experiments/blob/master/packages/my-app/src/notifications/extractDataFromBackgroundNotification.android.ts

[trajano]

also would this be backported to 51?