Enable use of assignment in the presence of accessors #2537
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.
Purpose
Allows specifying a fourth argument, an options bag, to
sandbox.replace()
to make it possible to override the default behaviour of throwing when encountering accessor methods (getters/setters).This means we will pass the supplied replacement to the setter and vice-versa get the original value from the getter when restoring.
P.S. I am not sold on the options name, so could be something else like "allowUsingAccessor". Was just inspired by the original issue.
Background (Problem in detail) - optional
Issue #2403 made the case for using assignment, instead of defining object descriptors, in injection methods. Enabling this in all existing methods would probably break some projects in unforeseen ways. It would also mean adding yet another argument to most of these, which would make them harder to use, as some of them (spy) already have optional third arguments today.
The discussion in #2403 revealed we already had a "special case" in our
Sandbox#replace
method, as this already uses basic assignment. The only thing preventing it from being used was that it tries to prevent false usage by throwing on encounter accessor methods for the given property. Thus this PR simply opens up for circumenting this.This opens up for some exotic cases, such as manual setups for stubbing the exports of true ESM modules through the innards of the module, in the way described in that feature:
my-module.mjs
my-module.test.mjs
Alternatively one could, of course, not do this change, and instead say that this manual stubbing setup is already doable by exposing functions for setting the corresponding exports and manually invoking these. The downside is that this will cause lots of manual restore code, which would otherwise by handled by Sinon.
How to verify - mandatory
Run tests and alternatively try the copy-pasting the example above. You will typically need to say
import sinon from '../lib/sinon-esm'
.Checklist for author
npm run lint
passes