GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Strict bind, call, and apply methods on functions by ahejlsberg · Pull Request #27028 · microsoft/TypeScript Conversation
|
| if (resolved === anyFunctionType || resolved.callSignatures.length || resolved.constructSignatures.length) { | ||
| const symbol = getPropertyOfObjectType(globalFunctionType, name); | ||
| const functionType = resolved === anyFunctionType ? globalFunctionType : | ||
| resolved.callSignatures.length ? globalCallableFunctionType : |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we prefer callable to newable if a given type is both?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I have a convincing argument for either preference. All I know is that it is very rare to have both and the two sets of signatures presumably align such that calling without new just causes the function to allocate an instance (as is the case with Array<T>, for example). I also know that having a combined list of overloads causes error reporting to be poor for one side because we always use the last arity-matching signature as the error exemplar.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Can we not use both sets of overloads for resolution, and if both fail, simply prefer one set or the other just when reporting errors?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
No, not easily. We assume that all overloads are in a single list of signatures, so we'd somehow have to merge them or declare a third type (CallableNewableFunction) in lib.d.ts that has the combined sets of overloads.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Correct me if I'm wrong, but won't the bind method overloads fail on functions with variable arguments? I use this kind of thing in my codebases... (not often, but they do exist) function foo(...numbers: number[]) {} const bar = [1, 2, 3]; foo.bind(undefined, ...bar); |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Yes, your example fails when interface CallableFunction extends Function { // ... bind<T, AX, R>(this: (this: T, ...args: AX[]) => R, thisArg: T, ...args: AX[]): (...args: AX[]) => R; } interface NewableFunction extends Function { // ... bind<AX, R>(this: new (...args: AX[]) => R, thisArg: any, ...args: AX[]): new (...args: AX[]) => R; } Should be pretty easy to add. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The second argument to |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
It's very important that it be an ArrayLike, since people .apply with NodeLists, arrays, and arguments objects. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Some part of me wishes this was not behind a flag. What’s an example of a usage of apply/bind that is correct but would break with this? If the two interfaces are only “injected” if the flag is on, how can libraries reference them without making an assumption on the consumer’s flag seeting? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
One initial comment, plus can you elaborate on the breaks that result from having this on everywhere as Felix Becker (@felixfbecker) requested? Or was that discussed during the last design meeting when I was out?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| } | ||
| if (resolved === anyFunctionType || resolved.callSignatures.length || resolved.constructSignatures.length) { | ||
| const symbol = getPropertyOfObjectType(globalFunctionType, name); | ||
| const functionType = resolved === anyFunctionType ? globalFunctionType : |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This should be put into a function and anyFunctionType’s declaration needs a comment saying “Use this function instead”.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Why is the variance change needed?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| if (!inferredType) { | ||
| const signature = context.signature; | ||
| if (signature) { | ||
| if (inference.contraCandidates && (!inference.candidates || inference.candidates.length === 1 && inference.candidates[0].flags & TypeFlags.Never)) { |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Why is this change needed?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The variance change is there to ensure we infer the most specific type possible when we have both co- and contra-variant candidates. Furthermore, knowing that an error will result when the co-variant inference is not a subtype of the contra-variant inference, we now prefer the contra-variant inference because it is likely to have come from an explicit type annotation on a function. This improves our error reporting.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| globalArrayType = getGlobalType("Array" as __String, /*arity*/ 1, /*reportErrors*/ true); | ||
| globalObjectType = getGlobalType("Object" as __String, /*arity*/ 0, /*reportErrors*/ true); | ||
| globalFunctionType = getGlobalType("Function" as __String, /*arity*/ 0, /*reportErrors*/ true); | ||
| globalCallableFunctionType = strictBindCallApply && getGlobalType("CallableFunction" as __String, /*arity*/ 0, /*reportErrors*/ true) || globalFunctionType; |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Comment here is also a good idea.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| handler: (...args: TS) => void, | ||
| ...args: TS): void; | ||
|
|
||
| call((x: number, y: number) => x + y); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Is this improved error because of the variance inference change?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Yes
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| error TS2318: Cannot find global type 'NewableFunction'. | ||
|
|
||
|
|
||
| !!! error TS2318: Cannot find global type 'CallableFunction'. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Probably shouldn’t get an error here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I didn't want to update the private lib.d.ts used by the tests as I suspect that'll cause some really noisy baseline changes.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
OK, just wanted to make sure this error wouldn't show up in some public-facing scenario.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There is currently no type-safe way to use an (myFunction as Function).apply(undefined, arguments); myFunction.apply(undefined, arguments as any); Note that you can still function printArguments() { Array.prototype.forEach.call(arguments, (item: any) => console.log(item)); } This is permitted because we only strictly check the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Felix Becker (@felixfbecker) There is a lot of code out there today that uses |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Anders Hejlsberg (@ahejlsberg) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Ha/yes -- I've never wanted that to have happened in my code when it has; be great to have some sort of warning about it! 😸 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
It looks like the new feature doesn't work well for generic or overloaded functions. Silly example: function foo<T>(name: string, arg: T): T { console.log(name); return arg; } // Type of `fooResult` is `{}`. :( let fooResult = foo.bind(undefined, "Matt")("TypeScript"); function bar(name: string, arg: number): number; function bar(name: string, arg: string): string; function bar(name: string, arg: string | number) { console.log(name); return (typeof arg === "number") ? arg + 1 : arg + "1"; } // Error: Argument of type '5' is not assignable to parameter of type 'string'. let barResult = bar.bind(undefined, "Matt")(5); It would be good to document this as a limitation in the "What's new in TypeScript" entry. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Is there a reason to not use 'unknown' instead of 'any' ? bind<T, A extends any[], R>(this: (this: T, ...args: A) => R, thisArg: T): (...args: A) => R; |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Anders Hejlsberg (@ahejlsberg) Is it possible to extend this functionality so that the compiler can detect errors of this type? Launching this code generates an error: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Grzegorz Szeliga (@szagi3891) Your example is covered by #7968; AFAICT it has nothing to do with the current issue because you aren't using |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
What is the solution in the case of using generic or overloaded functions? Is there going to be an updated version that will handle generic or overloaded functions? I'm trying to decide if I should pin to the previous working version of Typescript until you release a working version. Update: => Bypasses this for now, but ouch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
It's no worse than it was before, but yes, you're going to need a roundtrip through |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Is that also why the following doesn't work? export function binder< Method extends (this: Context, ...args: Args) => Result, Context, Args extends any[], Result >(method: Method, context: Context, ...args: Args) { return method.bind(context) } const context = { local: 'string value' } function fn(this: typeof context, arg: string) {} binder(fn, context) // [ts] Argument of type '(this: { local: string; }, arg: string) => void' is not assignable to parameter of type '(this: { local: string; }) => {}'. [2345] It would be nice if the error code 2345 was switched to something more specific about this. I ask because I am trying to figure out how to model the below in TypeScript (yet the above seems a hurdle in its accomplishment): function binder (method, context, …args) { const unbounded = method.unbounded || method const bounded = method.bind(context, ...args) Object.defineProperty(bounded, 'unbounded', { value: unbounded, enumerable: false, configurable: false, writable: false }) return bounded } The latter seems impossible to model in TypeScript correctly. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
James McParlane (@DrMiaow) Another option is |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.