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

Restore FileReaderProgressEvent #704

Merged
merged 1 commit into from
Jun 4, 2019
Merged

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Jun 3, 2019

This type doesn't exist, but it did before about a year ago, and is useful until we parametrise ProgressEvent with subtype properties.

Also fixes variable names in merge, which had source and target backward, making for some confusing reading.

Fixes microsoft/TypeScript#25510

This type doesn't exist, but is useful until we parametrise
ProgressEvent with subtype properties.

Fixes microsoft/TypeScript#25510
@sandersn
Copy link
Member Author

sandersn commented Jun 3, 2019

@saschanaz You might be interested in this one.

@sandersn sandersn requested review from andrewbranch and rbuckton June 3, 2019 22:01
@sandersn
Copy link
Member Author

sandersn commented Jun 3, 2019

@andrewbranch @rbuckton I'm adding you as reviewers so that somebody else on the team is aware of TSJS-lib-generator changes.

@saschanaz
Copy link
Contributor

Well, why not just parameterize it?

@@ -153,7 +153,7 @@ export function emitWebIdl(webidl: Browser.WebIdl, flavor: Flavor) {
/// Interface name to its related eventhandler name list map
/// Note:
/// In the xml file, each event handler has
/// 1. eventhanlder name: "onready", "onabort" etc.
/// 1. eventhandler name: "onready", "onabort" etc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@sandersn
Copy link
Member Author

sandersn commented Jun 4, 2019

@saschanaz This is easier for now until I understand the compatibility concerns of parametrising ProgressEvent. This definitely works the way people expect because that's the way it used to work.

@sandersn sandersn merged commit 17d343a into master Jun 4, 2019
@sandersn sandersn deleted the restore-FileReaderProgressEvent branch June 4, 2019 23:22
saschanaz added a commit to saschanaz/types-web that referenced this pull request Jun 5, 2019
saschanaz added a commit to saschanaz/types-web that referenced this pull request Jun 5, 2019
@fatcerberus
Copy link

What is meant by "parametrise ProgressEvent with subtype properties"? Making it into a discriminated union or something...?

saschanaz added a commit to saschanaz/types-web that referenced this pull request Jun 5, 2019
@sandersn
Copy link
Member Author

sandersn commented Jun 5, 2019

@fatcerberus See #707 for @saschanaz' implementation.

I was thinking of a general way to add properties, but #707 is sensibly much more restricted. It just lets subtypes change the type of target.

sandersn pushed a commit that referenced this pull request Jun 12, 2019
* Revert "Restore FileReaderProgressEvent (#704)"

This reverts commit 17d343a.

* Parameterize ProgressEvent

* use generic for existing types

* emit parameterized ProgressEvent

* use existing `type-parameters` field

* ensure attributeless events get type param in the future
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

Successfully merging this pull request may close these issues.

Regression in dom.lib.d.ts: TS complains about FileReader's result
4 participants