feat(jest-mock): allow jest.replaceProperty to replace undefined or nonexistent properties
#13958
Conversation
| throw new Error(`${String(propertyKey)} is not declared configurable`); | ||
| } | ||
| if (descriptor?.writable === false) { |
There was a problem hiding this comment.
writable check was missing. I bumped into it while playing with this case:
Hm.. What to do with non-writable values like process.version? Would be nice to replace them too. Perhaps via tolerateFrozen option? (Alternatives like tolerateNonWritable, or tolerateNonwritable don’t look convincing.)
| @@ -663,6 +663,22 @@ test('isLocalhost returns false when HOSTNAME is not localhost', () => { | |||
| }); | |||
| ``` | |||
|
|
|||
| Jest will make sure that the target property exists on the object and its value is not `undefined`. To skip this precaution, use `tolerateUndefined` option: | |||
There was a problem hiding this comment.
I don't really like the name tolerateUndefined. I don't have any better suggestions off the top of my head, tho.
tolerateMissing, allowMissing, setMissing... neither sounds good, but tolerates is even worse imo 😅
There was a problem hiding this comment.
What about respectUndefined? Not sure. Hm.. Did you see my comment on non-writable values? What about option strict: false which would allow creating undefined props and overwriting non-writable values? Simple thing.
The target property cannot be undefined and non-writable at the same time. So in a way one option which turns off precautions should be enough.
There was a problem hiding this comment.
Does replaceProperty actually disallow existing but undefined properties? I would consider this a bug.
And for non-existing properties, it probably makes more sense to add jest.addProperty instead which would act like replaceProperty + tolerateUndefined.
There was a problem hiding this comment.
Thanks for thinking together. How do you see replacement of non-writeable props? Should it just work or do we need an option?
There was a problem hiding this comment.
I think for mocking, replacing non-writeable is fine (though they should remain non-writeable). I view non-writeable more as a hint for the runtime behavior while testing setup comes before the actual runtime.
There was a problem hiding this comment.
I don't think I want addProperty - then I think we'd need a removeProperty as well. It might have been better for replaceProperty to have been called setProperty, then taken options for replacement strategy. Anyways, I think a single API on jest is enough.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
As discussed in #13496, it would be useful if
jest.replacePropertycould replaceundefinedor nonexistent properties as well.Here I implemented this through
tolerateUndefinedoption. The name can be different, of course.Test plan
Tests are added.