Fix DocumentFragment interface #199
Conversation
falsandtru
force-pushed
the
DocumentFragment
branch
2 times, most recently
from
8c52af5 to
57e9684
Compare
baselines/dom.generated.d.ts
Outdated
| @@ -3448,7 +3448,11 @@ declare var Document: { | |||
| new(): Document; | |||
| } | |||
|
|
|||
| interface DocumentFragment extends Node, NodeSelector, ParentNode { | |||
| interface DocumentFragment extends Node { | |||
| querySelector<K extends keyof ElementTagNameMap>(selectors: K): ElementTagNameMap[K] | null; | |||
There was a problem hiding this comment.
so why not extend from NodeSelector?
There was a problem hiding this comment.
As I specified in microsoft/TypeScript#14246, NodeSelector's properties are defined with ParentNode, not NodeSelector. So we must merge and remove NodeSelector by this pr and #201.
There was a problem hiding this comment.
I understand. but why remove NodeSelector from the extends clause, and then define the functions inline?
it would have been sufficient to just remove ParentNode from the extends clause.
There was a problem hiding this comment.
ok, I updated my PRs.
falsandtru
force-pushed
the
DocumentFragment
branch
from
57e9684 to
af0d602
Compare
|
Looking at the histry again, this change has been added in #137 by @YuichiNukiyama to address fix #4400, #7528. Reverting this change will be a breaking change. the ParentNode properties seem to be defined in Chrome, and Edge and referenced in the MDN documentation. This does not seem to be a worth while change, at least with the Selector API is still a working draft. |
|
This change doesn't affect selector apis. |
As I specified in microsoft/TypeScript#14223, ParentNode properties are NOT defined (implemented) in Edge. Of course Element should inherit ParentNode for its actual apis, but DocumentFragment is NOT. These interfaces have different state of implemetations about ParentNode. Do you really believe ParentNode apis on DocumentFragment? |
|
I am not an expert on web APIs and browser compt. What i care about is not introducing unneeded breaks for users of the standard library. The changes in #137 explicitly added And chrome: so unless I missed something, i am inclined to punt on this issue, and wait until the spec reaches the next stage before making any changes. |
|
So do you accept the error in Edge (to support the unclear spec)? What is your mind? I don't know about that. |
From a user perspective, it is easier to have the API and not use it, than to use it and get a compiler error. IE for example does not support many of the APIs in lib.d.ts today. |
|
I agree about that in general terms. But MDN says ParentNode properties are not experimenal in Element, experimetal in DocumentFragment. https://developer.mozilla.org/en-US/docs/Web/API/Element So I feel uneasy about the correctness of ParentNode apis in DocumentFragment. Finally, can you believe that correctness? |
I am not really an expert on web APIs, so i am not the best person to comment about the validity of these declarations. it seems that browsers do support them today; and i worry changing them would break valid code for no to little value. |
|
ok |
Fixes microsoft/TypeScript#14223