This should probably be in yourglobal gitignore too!

I've added it because the project explicitly instructs you to create settings for.vscode in theCONTRIBUTING.md.

Yeah I agree we should add it. Just wanted to let you know you can use a global gitignore file for it too.

It seems this means it is impossible to have keys in context that conflict withAsyncOptions ...

Shouldn't it delete entries fromoptions but not fromoptions.context ?

Given that this PR is breaking change maybe it should only support passing arguments throughoptions.context to bypass this processing ?

+1 for only passing arguments throughoptions.context. It'll be a pretty big change but for good reason. Still having the option to also use rest props onAsyncProps only complicates things.

I would also like to see thecontext arg onrun to be merged on top ofoptions.context.

@ghengeveld so just to be sure you want the api to change from:

<AsyncpromiseFn={loadPlayer}playerId={1}suspense={true}>useAsync(loadPlayer,{playerId:1,suspense:true});// AltuseAsync({promiseFn:loadPlayer,playerId:1,suspense:true});

To:

<AsyncpromiseFn={loadPlayer}context={{playerId:1}}suspense={true}>useAsync(loadPlayer,{context:{playerId:1},suspense:true});// AltuseAsync({promiseFn:loadPlayer,context:{playerId:1},suspense:true});

I think this would be a great change.

I'm also for making thecontext pure, as in nothing gets magically added to it or deleted from it. This will make the code more readable.

But I'd also recommend overloadinguseAsync in the future to allow:

useAsync(loadPlayer,{playerId:1},{suspense:true});

As I think this would result in the most readable api.

If we are changing APIs here anyways I'm all for moving from positional parameters to named parameters (one parameter with a keyed object).

So I'd prefer to be only left with

<AsyncpromiseFn={loadPlayer}context={{playerId:1}}suspense={true}>useAsync({promiseFn:loadPlayer,context:{playerId:1},suspense:true});

as that will make it more readable at a glance, allow for easier future expansion and will keep the typings more readable/less error-prone.

I'm in favor of dropping the overloaded signatures and only support the single object argument. The shorthand where you pass in the async function as first argument was added to support some very basic use cases where it'd be theonly argument, but it's not worth the extra complexity in code and API consistency.

By the way we're going to have to write a codemod for this, because it would otherwise require quite a bit of manual work to upgrade.

I'm also for making the context pure, as in nothing gets magically added to it or deleted from it. This will make the code more readable.

Agreed. People can always spread it themselves. This meansdeferFn will always only receive thecontext fromrun, and thecontext fromoptions is only passed topromiseFn.

I've updated the code so:

1. There is no more overload of useAsync.
2. There is no more magical context spreading / building.

This example no longer makes any sense if you're going to just remove the shorthand version, so this entire section should be removed.

Oxford comma.

What's the purpose thea property in this test?

Shouldn't this just be:

I haven't tested, but I think this may remove the need to explicitly cast toC in a few places.

The typing for this function is kinda wack.

First of all, theinit parameter is using aRequestInit type that doesn't seem to actually exist. I think the type parameterC here should replace every instance ofRequestInit... or rather, perhapsC should just be renamed toRequestInit here?

Second, the 3rd argument's type probably shouldn't beFetchOptions, but rather a subset ofFetchOptions (that may not need the 2nd type parameterC at all), because passing in an object containingpromiseFn ordeferFn props doesn't make any sense in this context, yet it's technically allowed by the types here.

I foundRequestInit defined innode_modules/@types/react-native/globals.d.ts. Appears to be a global type which should explain why it isn't declared in this file.
And I think C works just fine here as it's the type for theContext rather than the inits. So, it probably shouldn't replace any instance ofRequestInit.

Since the 2nd parameter's name starts with an underscore to indicate it isn't used, the 1st one should do the same for consistency/clarity.

Also, my other comment aboutC andRequestInit being the same thing applies here as well.