Fix flow typings of partitionObjectByKey

[lgeiger]

flow-bin@0.38.0 will throw the following error:

src/functional/partitionObjectByKey.js:25
 25:   return partitionObject(source, (_, key) => whitelist.has(key));
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ tuple type. This type is incompatible with the expected return type of
 24: ): Array<Object> {
        ^^^^^^^^^^^^^ array type

This PR fixes the flow typings of partitionObjectByKey to match partitionObject.

We discovered this problem in https://github.com/nteract/nteract/pull/1349 while trying to upgrade flow-bin.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[lgeiger]

It looks like this error isn't present in Node 4 (see Travis).
But I can reproduce the above error on my local machine using Node 7 and on Travis using Node 6.
It also has been reported in #215.

It would be great if someone could review this PR.

[rgbkrk]

/cc @knowbody - do you know if there's a way to raise this up for review?

[ide]

cc @zpao

[knowbody]

I tweeted @zpao he said that he will look into it https://twitter.com/matzatorski/status/824346183183323149

[zpao]

Internally we just have a $FlowFixMe here because upgrade codemods, but I'm curious what the right thing is. I tried to apply this change internally but some other code starts failing because we have some uses where Tv isn't consistent (primary use seems to be splitting a props object in a React component). So we can't take it as is. I don't know how to type that correctly though - mixed (instead of the type parameter) doesn't seem to work (have to add more type checks in places) and any is never awesome. Just changing the return signature to [Object, Object] to make sure the tuple is handled correctly (which seems to be the thing to fix in 0.38) and leaving the rest as it was seems to be fine. But it also seems overly generic.

cc @gabelevi to perhaps comment?

[ide]

It seems like the caller of this function might need to do a type cast. My guess is that Flow has to define Tv as the most generic supertype of all the object values, which is often something like mixed for React component props.

So the caller either should type cast the argument as Object or {[key: string]: any}, or type cast the returned partitions as {[key: string]: whateverMakesSense}.

Flow might be able to do something smart in this specific instance with $Keys, not sure if the complexity is worth it compared to using Object though.

[lgeiger]

I don't have a nice solution either, but it would work if we change partitionObject to the following and keep partitionObjectByKey unchanged:

function partitionObject(
  object: Object,
  callback: (value: string, key: string, object: Object) => boolean,
  context?: any
): Array<Object> {
...
}

[gabelevi]

Try [Object, Object]

Seems like a good compromise between the current troublesome annotation and the actual tuple type.

[gabelevi]

@zpao - can confirm that [Object, Object] worked internally, if you want to sync D4479141

[zpao]

@lgeiger - want to make it [Object, Object] which Gabe landed internally (to keep things mostly in sync)? Then I can ship it. Otherwise I'm happy to just make that change myself.

[lgeiger]

@zpao Sorry for the delay. I updated the PR 👍

[zpao]

Thanks! I just published v0.8.9 with this 🎉