Open
Conversation
This resolves warnings from npm regarding a number of moderate and high security vulnerabilities when running `npm audit`. This package-lock file is the automated result of running `npm audit fix`.
- Adds homepage, bugs, and repository fields.
Contributor
|
I haven't really dug in too deep but I appreciate having tests either way! 😄Unfortunately this kind of library is really hard to test properly without actual browser testing since it exploits some very funny behaviour of scroll events. You can look into Either way this is a great start! Thanks! 🎉 |
tchon
approved these changes
Jul 30, 2018
|
|
||
| afterEach(() => { | ||
| jest.clearAllMocks(); | ||
| wrapper.unmount(); |
There was a problem hiding this comment.
should we clear document.body.innerHTML here too as part of the teardown?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds
enzymeand an appropriate adapter to the devDependencies, and uses them to run a newly-added test suite.I wanted to avoid testing specific implementation details, instead focusing on the prop interface (ie, the onReflow/Resize/Position callback props), and ensuring that they were being called (or not called) in the appropriate scenarios.
Coverage is unfortunately not 100% just yet — I had some concerns around the handling of the actual calls to
add/removeEventListener, and recent changes that may be preventing the event listener from being appropriately cleaned up when the last component instance is unmounted that I think should be resolved first.I also needed to move onto some other tasks, but I felt it would be better to submit my work so far and revisit it later, rather than hold onto it until it's complete.
Changes
package.jsonfile. ad6496cenzyme&enzyme-adapter-react-16dependencies and updates other deps accordingly. 32db9e6, cd8cf8fnpm audit, which was resolved via the automatednpm audit fixcommand.removeSResizeListener→removeResizeListener. 72c2ad9