Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

type for Function.prototype.bind wrongly preserves fields on a function #34338

Open
RichieAHB opened this issue Oct 14, 2019 · 9 comments
Open
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@RichieAHB
Copy link

When calling bind on a function Javascript does not copy the properties across that have been added to that function, however TS treats them as having been copied.

TypeScript Version: 3.7-Beta

Search Terms: function bind

Code

const a = () => { }
a.hello = 'hi'

const b = a.bind({})

b.hello.split("") // Uncaught TypeError: Cannot read property 'split' of undefined

Expected behavior: hello field should not be present on typeof b.

Actual behavior: hello is deemed to exist on typeof b and the program crashes.

Playground Link: http://www.typescriptlang.org/play/?ts=3.7-Beta&ssl=1&ssc=1&pln=7&pc=1#code/MYewdgzgLgBAhjAvDAFASiQPhgbxgXwCg4A6ACwFMAbKkJGAcjIEsHDDRJYAje075mAAmKHPjTtu5arRIQADlWZQUAIlUYA9JpgBVMMDgBXAOZlYAFQCe8igFEATg5AOAXDADCcMGBCwHFHBCMPLOtg5QVowKSlAMMCAAZjBGwhSJghRChEA

Related Issues: #212 seems to be where this was discussed and finally implemented, although it's not directly related.

@ilogico
Copy link

ilogico commented Oct 14, 2019

BTW, this only happens when using this specific overload.

const f = (a: boolean) => a;
f.value = 42;

const g = f.bind(undefined);
g.value; // OK!

const h = f.bind(undefined, true);
h.value; // Error, as expected

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Oct 30, 2019
@RyanCavanaugh
Copy link
Member

Certainly not the correct behavior, but we'd need some way of expressing "call signatures only", which isn't expressable with the current type system primitives

@ilogico
Copy link

ilogico commented Oct 30, 2019

But it really looks like it's just a matter of rewriting the first overload.

bind<T, A extends any[], R>(this: (this: T, ...args: A) => R, thisArg: T): (...args: A) => R;

@RyanCavanaugh
Copy link
Member

bind today preserves overloads, which the proposed form wouldn't:

declare function fn(s: string): number;
declare function fn(n: number): string;

const b = fn.bind({});
// Both OK, but argument capture can't preserve overloads
b(10).toLowerCase();
b("").toFixed();

@RichieAHB
Copy link
Author

I actually made that change before realising it broke the overload tests, and I couldn’t find a way to preserve them, which I guess is your point in #34338 (comment)

Nice test coverage 👌

@RyanCavanaugh
Copy link
Member

Upon thinking about it more, it might be possible by intersecting with a mapped type that excludes non-Function keys to produce never.

That said, adding that extra complexity would need to be justified by someone running into this in practice. It seems like a somewhat difficult mistake to make. Was this encountered in an actual manifested bug, or just something you noticed?

@RichieAHB
Copy link
Author

RichieAHB commented Oct 31, 2019 via email

@RyanCavanaugh
Copy link
Member

type RemoveProps<T> = { [K in keyof T]: never };
type SafeBind<T> = T & RemoveProps<T>;

interface Function {
  bind2<T>(this: T, arg: any): SafeBind<T>;
}

function f(n: number): string;
function f(n: string): number;
function f(n: any) {
  return n;
}
f.prop = 0;

const t = f.bind2(null);
t(0);
t("");
t.prop; // never

@RichieAHB
Copy link
Author

T & was what I was missing, that looks like it would be exactly what was needed 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants