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

Improve method signatures returning Node type #220

Merged
merged 1 commit into from
Mar 29, 2017

Conversation

falsandtru
Copy link
Contributor

@@ -8249,20 +8249,20 @@ interface Node extends EventTarget {
readonly previousSibling: Node | null;
textContent: string | null;
appendChild<T extends Node>(newChild: T): T;
cloneNode(deep?: boolean): Node;
cloneNode(deep?: boolean): this;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will make all the Node hierarchy generic, something we have avoided for perf reasons. see #166 for more details.

The alternative here is for every interface that extends from Node, we can redefine cloneNode to return the containing type.

Copy link
Contributor

Choose a reason for hiding this comment

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

The other alternative is to just leave cloneNode the way it is and take the other changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I expected this fix to work as an easy workaround. Done.

@falsandtru
Copy link
Contributor Author

@mhegazy

@mhegazy mhegazy merged commit 077bd91 into microsoft:master Mar 29, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Mar 29, 2017

thanks!

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.

3 participants