Closed lallenfrancisl closed 2 years ago
@marchaos I know its kind of annoying to tag, but doing just in case this does not get lost in your notifications
Thanks. This did actually get lost in my notifications! Any chance you can add a test? That will help me understand the issue a bit more too.
Yeah working on some tests, at first I thought since this is type change tests wouldn't be needed. But this is something that needs to be tested to know if it works too
@marchaos I have added some unit tests. Some of the tests are actually duplicated from the mockDeep's existing tests, but changed to test everything works here too. I have added comments in the file to highlight this
Sweet thanks!, taking a look...
I pushed a new version 2.0.7
with this fix. Let me know if you've got any issues with that.
Thanks for the PR
Worth noting that this has introduced a performance issue, which is now resolved in the 3.0.x branch. This required an implementation change so that functions with prop support is now opt in. see issue https://github.com/marchaos/jest-mock-extended/issues/97 for more details. cc @lallenfrancisl
I haven't pushed a 3.0.0 final version yet, this is just a heads up.
Dang sorry, didn't expect that. I can also look into some alternative while avoiding performance issue. Will try to sit on this @marchaos
Hey @lallenfrancisl , I've already got an alternative for this:
mockDeep({ funcPropSupport : true })
This means that you only need to pass that in if you want to support functions with props / fields. For 99% of cases / people, this won't be required for deep mocking, but didn't want to remove that functionality. This is also why there will be a major version upgrade (3.0.0
). If you want to test it out before 3.0.0
is released, you can use 3.0.0-beta1
.
I saw the fix, but this still causes the issue when using the functionality, right ? I am not sure if I will get the time to test out this week. When are you planning to release 3.0 ?
Yes it does. Would be nice to get a perf fix for that, but inherently it's recursive in both branches so will be difficult to fix.
On Tue, 13 Sep 2022, 17:30 Allen Francis, @.***> wrote:
I saw the fix, but this still causes the issue when using the functionality, right ? I am not sure if I will get the time to test out this week. When are you planning to release 3.0 ?
— Reply to this email directly, view it on GitHub https://github.com/marchaos/jest-mock-extended/pull/95#issuecomment-1245659172, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVBY6OA5VWZUYXYELVEXD3V6CT3DANCNFSM53BRJ4KA . You are receiving this because you were mentioned.Message ID: @.***>
For some types like
AxiosInstance
the object (or rather better called as a function that has fields) doubles as function. This means that in the current type implementation for DeepMockProxy the if condition gets triggered for the function part. This causes the type to incorrectly say that only the first nested field is possible. To show this in action I am putting a simplified version similar to howAxiosInstance
behaves when used inside a class. You can find the original implementation ofAxiosInstance
hereThis PR fixes #94