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

HTMLElement onchange should use this: this #195

Open
ghost opened this issue Feb 15, 2017 · 5 comments
Open

HTMLElement onchange should use this: this #195

ghost opened this issue Feb 15, 2017 · 5 comments

Comments

@ghost
Copy link

ghost commented Feb 15, 2017

This example worked in 2.0, but not in 2.1:

const input = document.createElement('input');
input.value; // OK
input.onchange = function() {
    this.value; // Error
}

The current definition for onchange in HTMLElement is: onchange: (this: HTMLElement, ev: Event) => any;. Maybe it should use this: this?

@mhegazy
Copy link
Contributor

mhegazy commented Feb 17, 2017

Duplicate of microsoft/TypeScript#12490

I broke this with my change #166 to reduce instantiations in the library, by making less types generic (types using this are implicitly generic with respect to this).

The fix here is to generate onChange events for the type hierarchy with the correct type, instead of putting in the parent with type this.

@ghost
Copy link
Author

ghost commented Feb 17, 2017

If I'm reading it right, microsoft/TypeScript#14141 may fix this by setting the this type of the function to the type of input.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 18, 2017

If I'm reading it right, microsoft/TypeScript#14141 may fix this by setting the this type of the function to the type of input.

correct.

@myitcv
Copy link
Contributor

myitcv commented Mar 15, 2017

Given that microsoft/TypeScript#14141 has now been merged, what is left to do to fix this issue and microsoft/TypeScript#12490?

I note that with with 2.3.0-dev.20170315 and noImplicitThis the following still types incorrectly (per microsoft/TypeScript#12490):

let i = new Image();
i.onload; // HTMLElement.onload: (this: HTMLElement, ev: Event) => any

@saschanaz
Copy link
Contributor

saschanaz commented May 4, 2017

The fix here is to generate onChange events for the type hierarchy with the correct type, instead of putting in the parent with type this.

This won't help custom elements, and probably will generate much longer library file. (60+ lines per one HTMLElement interface, 80+ HTMLElements = 4800+ more lines)

Given that microsoft/TypeScript#14141 has now been merged, what is left to do to fix this issue and microsoft/TypeScript#12490?

The PR explicitly says that it works only on --noImplicitAny so I think it's not applicable to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants