-
Notifications
You must be signed in to change notification settings - Fork 113
Fix updateProps Function #170
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
Conversation
Codecov Report
@@ Coverage Diff @@ ## master #170 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 1 1 Lines 90 88 -2 Branches 28 28 ========================================= - Hits 90 88 -2
Continue to review full report at Codecov.
|
bc7c1c9 to
ddd4cc8
Compare
Hi! Thank you very much for this 🙌 It would be great if you could add a test that would fail with the previous implementation and that passes with the proposed one, so that we could easily see the issue and how it is prevented :)
I know the code change, but I don't seem to be able to run a check locally. After running an npm install with the next branch, I haven't been able to run tests properly on master. Also, I noticed that for some reason package-lock.json files aren't generated when an install occurs.
* Changed updateProps to return the Promise created from wrapper.setProps. * Updated the `updateProps` test to make sure the function can be called multiple times without producing errors.
ddd4cc8 to
8d8aee2
Compare
@afontcu Done.
Regarding my previous comment, it seems that
- Performing
npm installonmaster(or a branch on top of it) - Then performing
npm installonnext(or a branch on top of it)
Will cause problems to happen -- at least on certain machines. And the problem is that future installs on the master branch (or "related" branches) will permanently be broken, causing attempted tests to end in failure frequently. The next branch still seems to be able to run its tests just fine.
I'm running Ubuntu 18.04. After installing on master, then next, I was unable to perform a proper install on master anymore. More specifically, after performing an install on the master branch, I would still be unable to run any tests successfully. I can't tell if this is an issue with the master branch, the next branch, my machine, or some combination in between. Something could be wrong with the packages too? But it complicates contributing because switching between next and master branches requires me to use 2 different local versions of the forked repo. I think it's something worth investigating.
Hi!
I know the code change, but I don't seem to be able to run a check locally. After running an npm install with the next branch, I haven't been able to run tests properly on master.
next branch is using Vue 3 and I recently merged some changes regarding types that might break that branch. That's OK, I'll be fixing them soon.
Also, I noticed that for some reason package-lock.json files aren't generated when an install occurs.
This is intentional, see https://github.com/testing-library/vue-testing-library/blob/master/.npmrc
That makes sense. Thanks!
Thank you! 🤗
The Problem
I just discovered a bug in one of my projects with the
updatePropsfunction returned fromrender. WheneverupdatePropsis called multiple times, several errors are logged in the console and Jest is prevented from finishing its test run. This is a huge bug for obvious reasons. Anyone can reproduce this error by grabbing the (currently) latest version of VTL and making multipleawaited calls toupdatePropsin a test for a Vue component.The logged errors seem to be related to asynchronicity. I think that since the
Promisereturned fromupdatePropsis different from the one returned fromwaitFor, everything gets thrown out of balance when the developer tries to run tests with multipleupdatePropscalls.The fix I'm proposing simply returns the
Promiseobtained fromwrapper.setPropsdirectly. I applied this fix to mynode_modules, and my tests behaved properly afterwards.Changes