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

Update lib dom for 4.2 #42067

Merged
merged 1 commit into from
Jan 4, 2021
Merged

Update lib dom for 4.2 #42067

merged 1 commit into from
Jan 4, 2021

Conversation

orta
Copy link
Contributor

@orta orta commented Dec 21, 2020

Updates the dom.d.ts files - Key changes:

  • Some of the big DOM querying APIs default to HTMLElements (shoudn't be a breaking change)
  • Some of the DOM query APIs are now generic, so they can work with SVG or XHTML instead of just HTML DOMs
  • import.meta.url is now defined

Breaking changes

  • DOM APIs which are deprecated, are now marked as deprecated
  • The Streams API had some fields removed which aren't used in any browsers

microsoft/TypeScript-DOM-lib-generator#960

@orta orta self-assigned this Dec 21, 2020
@orta orta requested a review from sandersn December 21, 2020 18:43
@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Dec 21, 2020
@orta
Copy link
Contributor Author

orta commented Jan 4, 2021

OK, merging this at the end fo the day and I'll be keeping an eye out over the week for changes

@orta orta merged commit 222f29f into microsoft:master Jan 4, 2021
@ahejlsberg
Copy link
Member

This PR breaks a lot of DT tests. In particular, lots of noise about modifiers of some url property not matching. Also, there's this new definition of getElementById:

getElementById<E extends Element = HTMLElement>(elementId: string): E | null;

Type parameters that aren't used in the parameter list are an anti-pattern. Using a type parameter only in the return type basically amounts to an implicit type cast to the contextual type, if any. In this case the change is also breaking because the new definition produces Element when inference from the contextual type infers a type that isn't assignable to Element, such as:

declare function getElementById<E extends Element>(s: string): E | null;
declare function foo(x: string | HTMLElement): void;
foo(getElementById('hello'));  // Now errors

What happens here is that we infer string | HTMLElement for E and since that isn't assignable to Element, we revert to the constraint Element, which then fails.

@ahejlsberg
Copy link
Member

BTW, we typically run the DT tests before merging PRs like this. Having lots of breaks in the DT tests in master leads to a bunch of counter productive noise in other PRs.

@orta
Copy link
Contributor Author

orta commented Jan 5, 2021

Oof, yeah, my bad - sorry, my first time sending them over from the DOM repo.

I can revert tomorrow and look at extracting the Element changes out to get a safer merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants