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

Breaking change in 2.1: Node.firstChild can now be null #12720

Closed
felixfbecker opened this issue Dec 7, 2016 · 5 comments
Closed

Breaking change in 2.1: Node.firstChild can now be null #12720

felixfbecker opened this issue Dec 7, 2016 · 5 comments
Labels
Breaking Change Would introduce errors in existing code Docs The issue relates to how you learn TypeScript

Comments

@felixfbecker
Copy link
Contributor

felixfbecker commented Dec 7, 2016

TypeScript Version: 2.1.4

This code snippet used to compile fine in 2.0 with strictNullChecks, but now errors in 2.1:

if (documentElement.hasChildNodes() && documentElement.firstChild.nodeName === 'error') {
}

I agree that firstChild can in fact be null (even though in this case I checked with hasChildNodes), but this is a breaking change introduced in a minor version.

Workaround: check with documentElement.firstChild && or add a ! in the cases where I know from the protocol that the element will have a child node.

Full file: https://github.com/felixfbecker/vscode-php-debug/blob/master/src/xdebugConnection.ts
Failing build: https://travis-ci.org/felixfbecker/vscode-php-debug/builds/182024998

@blakeembrey
Copy link
Contributor

Not sure if this is a good workaround, but also:

declare interface Node {
    hasChildNodes(): this is this & { firstChild: Node, lastChild: Node }
}

@felixfbecker
Copy link
Contributor Author

That would be an improvement, but it's still a breaking change, so please at least document it. I solved this already in xdebug/vscode-php-debug@0ccad83

@gcnew
Copy link
Contributor

gcnew commented Dec 7, 2016

I dont see this as a breaking change - this is not the compiler at work, it is the DOM definitions. They get improved every day and sometimes new errors get caught because of the changes. Previous definitions were just incomplete and thats getting fixed :)

PS: If I had stumbled on firstChild's 2.0 type I'd have been rather baffled by its incompleteness

@mhegazy
Copy link
Contributor

mhegazy commented Dec 7, 2016

here is the original issue that this change fixed: #11113
The PR with the change: https://github.com/Microsoft/TSJS-lib-generator/pull/149/files

Now added to the breaking change section for this release: https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#changes-to-dom-apis-in-the-standard-library

@mhegazy mhegazy added Docs The issue relates to how you learn TypeScript Breaking Change Would introduce errors in existing code labels Dec 7, 2016
@felixfbecker
Copy link
Contributor Author

Thanks @mhegazy. I think it does not make sense to release a new major here because in a way it could be considered a bug in my code (even though as shown I am doing appropriate checks and am also working with parsing a well-defined protocol).

@mhegazy mhegazy added this to the TypeScript 2.1 milestone Dec 7, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Would introduce errors in existing code Docs The issue relates to how you learn TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants