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

Tests are a nice thing to have :) #13

Open
AmirTugi opened this issue Jun 16, 2020 · 2 comments
Open

Tests are a nice thing to have :) #13

AmirTugi opened this issue Jun 16, 2020 · 2 comments

Comments

@AmirTugi
Copy link

AmirTugi commented Jun 16, 2020

Hi hi
I was looking at this lib (got here through a post in Medium)
And it looks really good and simple.
The only thing that I'm missing in order to put in my production app is the testing.
I need to guarantee that the component will not break my app, or that accidentally the usage API has been changed.
The tests will make sure nothing breaks, and if the usage is changed - this will help indicate that probably a major version needs to change.
(I currently don't have the time to help with the testing implementation itself, but feel free to contact me for guidance or moral support :D)

Well done on the lib 👍

@gkaemmer
Copy link
Owner

It's a great point, and I totally agree with you.

I don't have tests for this mostly because it's a 50 line file that was written a few years ago and has barely changed since. Quite frankly, the surface area is tiny, I don't worry much about things going wrong, and writing tests seems like overkill. For what it's worth, I run the example server and do visual QA every time I push an update. But your skepticism is correct here.

Because its so small, I'd encourage you (and those who see this in the future) to consider copying the contents of FadeIn.js into your project directly instead of adding this to your package.json. For the reasons you outlined (what if something breaks?), we should probably all be more wary of pulling in tiny dependencies for one-off features.

@AmirTugi
Copy link
Author

Thanks. It's exactly what we have done.
Works great :)

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

No branches or pull requests

2 participants