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

Fix check of node type #145

Merged
merged 2 commits into from
Aug 27, 2019

Conversation

Benjaminsson
Copy link
Contributor

Closes #137

Seems that enzyme-adapter-react-16 was the one that broke the type checks. So this also bumps those versions to make sure the tests passes both before and after the bump.

@VincentLanglet
Copy link
Collaborator

Hi @Benjaminsson, thanks for your PR.
Can you add a test for the case you fix ?

In version 1.14.0 of enzyme-adapter-react-16 this check does not work.
@codecov
Copy link

codecov bot commented Aug 27, 2019

Codecov Report

Merging #145 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #145   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           6      6           
  Lines         107    107           
  Branches       41     41           
=====================================
  Hits          107    107
Impacted Files Coverage Δ
src/utils.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24d2c11...a79dc3d. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Aug 27, 2019

Codecov Report

Merging #145 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #145   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           6      6           
  Lines         107    107           
  Branches       41     41           
=====================================
  Hits          107    107
Impacted Files Coverage Δ
src/utils.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24d2c11...a79dc3d. Read the comment docs.

@Benjaminsson
Copy link
Contributor Author

@VincentLanglet I added a test case. I don't know how much value it gives. As you can see the test coverage was already 100% and it did actually break when updating the adapter. But might be a bit more future proof now? I'm not super experienced in testing but this should do the job right?

@VincentLanglet
Copy link
Collaborator

@Benjaminsson It's a good way for me to be sure the PR does the job.
And it avoid futur regressions.

Thanks !

@VincentLanglet VincentLanglet merged commit a8f9fcf into adriantoine:master Aug 27, 2019
@mycolaos
Copy link

Relating to the original issue #137, I'm wondering what will be printed in the snapshot? React.memo in place of Component? Shouldn't it be Memo(ComponentDisplayName)?

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.

Component created using React.memo appears as "Component" in snapshot
3 participants