flow
flow copied to clipboard
facebook
Fix Function.prototype.apply.bind
Fixes #5681
const fn = (a: number) => {}
export const fnApply = fn.apply.bind(fn) // no error
Also adds argument to Function$Prototype$Apply
declare var fnApply: Function$Prototype$Apply<typeof fn>
A silly question: What significance does binding the apply function itself have? Can you share an example where the observable effect of checking is different?
If it is significant, I think the solution of adding a this type to the internal representation of ApplyT is reasonable. However, I don't think we should add a type parameter to the annotation syntax, or at least not until the overall this-type story is sorted.
@samwgoldman it would be possible to apply $Compose or other custom functions on type-level
function spread(fn) {
return Function.apply.bind( fn, null );
}
spread(fn)([1, 2, 3]) // fn(1, 2, 3)
https://github.com/lttb/flown/blob/master/src/Reduce/index.js
type Apply<Fn, Args> = $Call<Function$Prototype$Apply<Fn>, null, Args>
// basically $Call with tuple
declare function reduce<Fn, A, T>(Fn, A, T): $Call<
Apply<
$ComposeReverse,
$TupleMap<T, Fn>,
>
, A>
export type _Reduce = typeof reduce
export type Reduce<Fn, A, T> = $Call<_Reduce, Fn, A, T>
type MergeAll<T> = Reduce<<X, V>(V) => X => { ...$Exact<X>, ...$Exact<V> }, {}, T>
declare function mergeAll<A>(A): MergeAll<A>
const x = mergeAll([{ a: 1 }, { b: 1 }, { c: 1 }, { a: 2 }])
;(x: { a: 2, b: 1, c: 1 })
/**
* $ExpectError
*
* the type of 'c' is incompatible with number
*/
;(x: { a: 2, b: 1, c: '1' })
@goodmind I don't see how these examples are related to the linked issue, or Function.prototype.apply/bind. I would much prefer to focus on the problem at hand, since there's enough work involved in supporting the use case from Marshall's example.
- I still don't think we should add a type argument to Function$Prototype$Apply. Even if it makes some things expressible, it's putting the cart before the horse.
- Per Marshall's example, adding the this type to the internal FunApplyT repr isn't enough, we also need to add the partial argument list.
- The implementation here does not look correct (will comment inline) so some tests are definitely needed.
@samwgoldman now it should work, I didn't expose partial apply to arguments itself, only to use_t
@samwgoldman how it would work then for this case
type Apply<Fn, Args> = $Call<Function$Prototype$Apply<Fn>, null, Args>
We don't need it to fix #5681. If you want to support another case, I think that you should write up a more thorough proposal. For example, why should we support that?
@samwgoldman maybe I can remove argument if you don't want it and decide later how to make my case work? (or write proposal)
Yeah. I would want to have a full understanding of your use case before adding something. It's possible that another solution would be better. I'm not opposed to adding something later, but we need to follow a process to make sure that we're adding the right things for the right reasons.
Probably related issue, methods doesn't have function prototype https://flow.org/try/#0MYGwhgzhAECC0G8BQ1oQC5nQS2NAZgBRgBc0AdgK4C2ARgKYBOAlIitAL5JdKwB0+PmAAOwkAE9CVECAA00ANoByJQF1mQA