Repository navigation
Fix getters being imitated as undefined, and td.replace on getter properties - #550
Merged
Merged
Conversation
copyProps built every fake property from `descriptor.value`, so any accessor property, which has `get` instead, came out as `undefined`. That broke td.replace(path), td.object, td.constructor and td.imitate for objects with getters. Most visibly, SWC compiles every export to a getter, and Babel and tsc compile re-exports to getters, so those modules' exports couldn't be stubbed. Pass the original object down to copyProps and call each getter on it, imitating the result. Fall back to `undefined`, as before, when: - the getter (or imitating its result) throws - there's only a setter - the object is a prototype, which has no instance to call it on - getters are nested more than 3 deep, since a getter can return a fresh object with getters of its own, forever
td.replace(object, property) swapped the property by assignment, both when replacing and when restoring on reset. Assigning to a getter with no setter throws in strict mode, so it failed with "Cannot set property ... which has only a getter". For a getter with no setter, define the fake as a data property instead: - An own, configurable getter (as tsc defines re-exports) is redefined, and reset puts the original descriptor back exactly. - An inherited getter (a class getter, replaced through an instance) is shadowed by an own property, which reset deletes. - An own, non-configurable getter (as SWC defines exports) can't be redefined, so it gets an error that points to replacing the whole module with td.replace(path) instead. Data properties and getters with setters are still replaced by assignment, as before.
Babel compiles `export { square } from './squarer'` to a getter, which
td.replace(path) used to imitate as `undefined`. This runs that path
through a real compiler in CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #526
The problem
Any property defined with a getter came out of imitation as
undefined. The property copier read onlydescriptor.value, and accessor descriptors havegetinstead. That affected every entry point that imitates:td.replace(path)(including testdouble-jest'std.mock),td.object,td.constructorandtd.imitate.The most visible case is compiled CommonJS:
@swc/jest) couldn't stub their own modules at all:td.when(dep.doTheThing())threwdep.doTheThing is not a function.tsccompile re-exports (export { x } from,export *) to getters, so barrel files broke the same way.td.replaceEsmwas unaffected, because namespace objects use data properties.A related bug:
td.replace(object, 'prop')replaced and restored by assignment. That throwsCannot set property ... which has only a getterwhenpropis a getter with no setter.What changed
Imitating getters (
copy-props.js,overwrite-children/index.js)The original object is now passed down to
copyProps, which calls each getter on it and imitates the result. Functions become test doubles and objects are imitated recursively, as for data properties. It falls back toundefined, which was the old behavior, when:td.constructor(Foo)instances still read prototype getters asundefinedget kid () { return new A() }), so the depth is capped to keep imitation finiteReplacing getter properties (
replace/property.js)For a getter with no setter,
td.replace(object, 'prop')now defines the fake instead of assigning it:td.reset()tscre-exports)td.replace('path/to/module')Data properties and getters with setters are still replaced by assignment, as before.
Behavior changes
fsorcrypto, or lazy loading. Getters that throw still giveundefined.td.replace(moduleObject, 'export')can't work on SWC output, because JavaScript won't let a non-configurable getter be redefined. It now fails with an error explaining that and suggestingtd.replace(path), which does work for these modules after this change.Review notes
finallyrestores the counter even when something throws. Its one quirk: a getter that callstd.imitateitself starts that inner imitation partway up the cap, so it gets fewer levels of expansion.descriptor.valuespecifically to avoid running getters. The hazards it named are handled elsewhere now:gatherPropsstops atFunction.prototypeandObject.prototype.arguments/callerdata properties.try/catchcovers anything that still throws.