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

Regression in dom.lib.d.ts: TS complains about FileReader's result #25510

Closed
vvs opened this issue Jul 8, 2018 · 14 comments · Fixed by microsoft/TypeScript-DOM-lib-generator#704
Assignees
Labels
Bug A bug in TypeScript Needs Investigation This issue needs a team member to investigate its status.

Comments

@vvs
Copy link

vvs commented Jul 8, 2018

TypeScript Version: 3.0.0-dev.20180707

Search Terms: dom, FileReader

Code

Taken directly from MDN: https://developer.mozilla.org/en-US/docs/Web/API/FileReader/onload

function onChange(event) {
    var file = event.target.files[0];
    var reader = new FileReader();
    reader.onload = function(event) {
      // The file's text will be printed here
      console.log(event.target.result)
    };
  
    reader.readAsText(file);
  }

Expected behavior:
No errors.

Actual behavior:
[ts] Property 'result' does not exist on type 'EventTarget'.


This is a regression, since there were no such errors reported in TS 2.9.2.

@vvs
Copy link
Author

vvs commented Jul 8, 2018

It looks like the regression was introduced in 7a7d04e

@mhegazy
Copy link
Contributor

mhegazy commented Jul 10, 2018

The behavior change was introduced in microsoft/TypeScript-DOM-lib-generator#448. the PR switched to use the widl file to generate the .d.ts file for the File APIs. The widl does not seem to define the FileReaderProgressEvent (which was defined in the past). It is not clear if this is a spec bug, or an intentional change to the spec.

//cc @saschanaz

@mhegazy mhegazy added Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Needs Investigation This issue needs a team member to investigate its status. labels Jul 10, 2018
@saschanaz
Copy link
Contributor

FileReaderProgressEvent is obsolete and replaced by ProgressEvent, as the PR says.

@vvs
Copy link
Author

vvs commented Jul 10, 2018

Interesting. So the spec intent is clear: the load event is fired on the context object, which is a FileReader, and the event's target points to the FileReader, so it has the result field.

The current declarations don't capture this, stating that we deal with a generic ProgressEvent, and its target property is naturally doesn't point to FileReader, and it doesn't have the result, which is a regression from the older declarations.

@saschanaz
Copy link
Contributor

Yeah... I think this is related to #299.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 10, 2018

@saschanaz maybe we should redefine the FileReaderProgressEvent for now.

@bildja
Copy link

bildja commented Aug 3, 2018

Hi, any news on this? After updating typescript to 3.0 I am getting Cannot find name 'FileReaderProgressEvent'.

@saschanaz
Copy link
Contributor

I would suggest using general ProgressEvent as no major browser supports FileReaderProgressEvent, including Edge, Firefox, and Chrome. @mhegazy, or maybe microsoft/TypeScript-DOM-lib-generator#560 should have covered this?

@rch850
Copy link

rch850 commented Aug 17, 2018

Hi. I introduced FileReaderProgressEvent in microsoft/TypeScript-DOM-lib-generator#398 and found this issue today.

It is possible to have event.targer.result without FileReaderProgressEvent. The first commit microsoft/TypeScript-DOM-lib-generator@90971b1 of the PR was implemented in such way. Can we use this?

@saschanaz
Copy link
Contributor

It is possible to have event.targer.result without FileReaderProgressEvent. The first commit microsoft/TypeScript-DOM-lib-generator@90971b1 of the PR was implemented in such way.

I think microsoft/TypeScript-DOM-lib-generator#207 is better as its more general, readable, and easy to write. We may add ProgressEvent<T> first and see how things go.

@elf-pavlik
Copy link

Any progress on this one?

@G-Rath
Copy link

G-Rath commented Dec 18, 2018

Any progress on this? What are we waiting on to get this fixed, as it's clearly been identified as a bug that needs fixing...

@mychalhackman
Copy link

For now I worked around this with:

reader.onload = (event: ProgressEvent & FileReaderEvent) => { ...

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript and removed Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript labels Mar 25, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.5.0 milestone Mar 25, 2019
@sandersn
Copy link
Member

It's too late to fix this for 3.5 but it needs to be fixed as soon as 3.6 development starts so people have a chance to test the new lib.dom.d.ts.

sandersn added a commit to microsoft/TypeScript-DOM-lib-generator that referenced this issue Jun 3, 2019
This type doesn't exist, but is useful until we parametrise
ProgressEvent with subtype properties.

Fixes microsoft/TypeScript#25510
sandersn added a commit to microsoft/TypeScript-DOM-lib-generator that referenced this issue Jun 4, 2019
This type doesn't exist, but is useful until we parametrise
ProgressEvent with subtype properties.

Fixes microsoft/TypeScript#25510
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants