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

Node.parentElement should be maybe-null #11113

Closed
evmar opened this issue Sep 23, 2016 · 3 comments
Closed

Node.parentElement should be maybe-null #11113

evmar opened this issue Sep 23, 2016 · 3 comments
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this

Comments

@evmar
Copy link
Contributor

evmar commented Sep 23, 2016

lib.dom.ts defines Node.parentElement as never-null:
https://github.com/Microsoft/TypeScript/blob/95c3eccbe9f3f7e1857ce157d437e6b80b3c9c6f/lib/lib.dom.d.ts#L8749

However, it's a pretty common pattern to write loops like

while (n.parentElement != null) {
  // [...]
  n = n.parentElement;
}

to walk up the node tree.

Also parentNode, previousSibling, nextSibling, firstChild, lastChild... (?)

@mhegazy mhegazy added Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Bug A bug in TypeScript Help Wanted You can do this labels Sep 23, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Sep 23, 2016

previousSibling, nextSibling, firstChild, lastChild... etc.. are all fair game. please feel free to send us a PR for that. You can find more information about contributing lib.d.ts fixes at https://github.com/Microsoft/TypeScript/blob/master/CONTRIBUTING.md#contributing-libdts-fixes.

parentElement is a tricky one. All elements except document have parent elements, so it is going to be annoying to most ppl. so i am not sure here what is best option, either correctness or usability.

@mhegazy mhegazy added this to the Community milestone Sep 23, 2016
@kitsonk
Copy link
Contributor

kitsonk commented Sep 24, 2016

All elements except document have parent elements,

If they are in the DOM. It is potentially a way to identified DOM fragments and other scenarios when moving parts of the DOM about. Strict is strict, (except when it isn't!). 😉

@mhegazy
Copy link
Contributor

mhegazy commented Sep 25, 2016

I would defer to @kitsonk opinion on this. I do not really write a lot of DOM code, so I would have limited insight here.

falsandtru added a commit to falsandtru/TypeScript that referenced this issue Oct 1, 2016
@mhegazy mhegazy closed this as completed Oct 3, 2016
@mhegazy mhegazy modified the milestones: Community, TypeScript 2.1 Oct 3, 2016
@mhegazy mhegazy reopened this Oct 3, 2016
@mhegazy mhegazy added Fixed in TSJS repo Fix merged in https://github.com/Microsoft/TSJS-lib-generator, but not ported yet and removed Fixed in TSJS repo labels Oct 21, 2016
@mhegazy mhegazy modified the milestones: TypeScript 2.1, TypeScript 2.1.2 Oct 27, 2016
@mhegazy mhegazy added Fixed A PR has been merged for this issue and removed Fixed in TSJS repo Fix merged in https://github.com/Microsoft/TSJS-lib-generator, but not ported yet labels Oct 31, 2016
@mhegazy mhegazy added the Breaking Change Would introduce errors in existing code label 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 Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

4 participants