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.

Sign up for GitHub

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

Jump to bottom

fix cloneElement test #982

Merged
merged 2 commits into from Aug 17, 2018
Merged

fix cloneElement test #982

merged 2 commits into from Aug 17, 2018

Conversation

decadef20
Copy link
Contributor

@decadef20 decadef20 commented Jan 20, 2018 •
edited

The clone and instance are both VNode instances ,why to test prototype property here?
Maybe we should test whether clone.nodeName.prototype equals to instance.nodeName.prototype ,or I missed something?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 1a123cc on decadef20:master into 66d74c5 on developit:master.

@developit
Copy link
Member

Hmm - I think the expect() there is actually just verifying that both are VNode instances. A constructor check would seem much more appropriate.

@decadef20
Copy link
Contributor Author

Great. I also think so.

@developit developit merged commit f98df92 into preactjs:master Aug 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@developit developit developit approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@decadef20 @coveralls @developit