Skip to content

Commit 769ea69

Browse files
committed
Replace getters with td.replace(object, property)
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.
1 parent a344b26 commit 769ea69

3 files changed

Lines changed: 172 additions & 2 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@
3333
`td.constructor` and `td.imitate`. Most visibly, every export of a module
3434
compiled by SWC, and re-exports compiled by Babel or `tsc`, couldn't be
3535
stubbed [#526](https://github.com/testdouble/testdouble.js/issues/526)
36+
* Fix `td.replace(object, 'property')` throwing `Cannot set property ... which
37+
has only a getter` when the property is a getter with no setter. It now
38+
replaces the getter, and `td.reset()` restores the original getter. A getter
39+
that can't be redefined (as SWC defines exports) gets an error that suggests
40+
`td.replace('path/to/module')` instead
41+
[#526](https://github.com/testdouble/testdouble.js/issues/526)
3642
* **Behavior change:** when imitating an object, testdouble now calls its
3743
getters and imitates what they return, instead of using `undefined`. Any side
3844
effects of those getters now run. Getters that throw, getters on prototypes

‎src/replace/property.js‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,23 @@ export default function (object, property, manualReplacement) {
1010
Object.prototype.hasOwnProperty.call(object, property)
1111

1212
if (isManual || realThingExists) {
13+
const getter = findGetterOnly(object, property)
14+
if (getter && getter.isOwn && !getter.descriptor.configurable) {
15+
return log.error('td.replace', `"${property}" is defined with a getter that cannot be redefined. Compilers like SWC and Babel define ES module exports this way. To replace a module's export, replace the whole module with td.replace('path/to/module') instead.`)
16+
}
1317
const realThing = object[property]
1418
return _.tap(getFake(isManual, property, manualReplacement, realThing), (fakeThing) => {
15-
object[property] = fakeThing
19+
if (getter) {
20+
defineValue(object, property, fakeThing, getter.descriptor.enumerable)
21+
} else {
22+
object[property] = fakeThing
23+
}
1624
reset.onNextReset(() => {
17-
if (realThingExists) {
25+
if (getter && getter.isOwn) {
26+
Object.defineProperty(object, property, getter.descriptor)
27+
} else if (getter) {
28+
delete object[property]
29+
} else if (realThingExists) {
1830
object[property] = realThing
1931
} else {
2032
delete object[property]
@@ -26,6 +38,30 @@ export default function (object, property, manualReplacement) {
2638
}
2739
}
2840

41+
// Assigning to a getter with no setter throws, so find any such accessor
42+
// (on the object or up its prototype chain) to replace it by definition
43+
const findGetterOnly = (object, property) => {
44+
let thing = object
45+
while (thing != null) {
46+
const descriptor = Object.getOwnPropertyDescriptor(thing, property)
47+
if (descriptor) {
48+
return descriptor.get && !descriptor.set
49+
? { descriptor, isOwn: thing === object }
50+
: undefined
51+
}
52+
thing = Object.getPrototypeOf(thing)
53+
}
54+
}
55+
56+
const defineValue = (object, property, value, enumerable) => {
57+
Object.defineProperty(object, property, {
58+
configurable: true,
59+
writable: true,
60+
enumerable,
61+
value
62+
})
63+
}
64+
2965
const getFake = (isManual, property, manualReplacement, realThing) => {
3066
if (isManual) {
3167
warnIfTypeMismatch(property, manualReplacement, realThing)

‎test/safe/replace.test.js‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,134 @@ module.exports = {
149149
assert._isEqual(Object.prototype.hasOwnProperty.call(dependency, 'notAThing'), false)
150150
}
151151
},
152+
'Replacing a property defined with a getter': {
153+
'own and configurable (tsc re-exports)': {
154+
beforeEach () {
155+
Object.defineProperty(dependency, 'beep', {
156+
configurable: true,
157+
enumerable: true,
158+
get: () => () => 'og beep'
159+
})
160+
},
161+
'using automatic replacement' () {
162+
const ogDescriptor = Object.getOwnPropertyDescriptor(dependency, 'beep')
163+
164+
const double = td.replace(dependency, 'beep')
165+
166+
assert._isEqual(td.explain(double).isTestDouble, true)
167+
assert._isEqual(dependency.beep, double)
168+
169+
// phase 2: reset puts the getter itself back
170+
td.reset()
171+
assert._isEqual(Object.getOwnPropertyDescriptor(dependency, 'beep'), ogDescriptor)
172+
assert._isEqual(dependency.beep(), 'og beep')
173+
},
174+
'with manual replacement' () {
175+
const ogDescriptor = Object.getOwnPropertyDescriptor(dependency, 'beep')
176+
177+
const fake = td.replace(dependency, 'beep', () => 'fake beep')
178+
179+
assert._isEqual(dependency.beep, fake)
180+
181+
// phase 2: reset puts the getter itself back
182+
td.reset()
183+
assert._isEqual(Object.getOwnPropertyDescriptor(dependency, 'beep'), ogDescriptor)
184+
}
185+
},
186+
'inherited from a class': {
187+
beforeEach () {
188+
class Horn {
189+
get beep () { return () => 'og beep' }
190+
}
191+
dependency = new Horn()
192+
},
193+
'using automatic replacement' () {
194+
const double = td.replace(dependency, 'beep')
195+
196+
assert._isEqual(td.explain(double).isTestDouble, true)
197+
assert._isEqual(dependency.beep, double)
198+
assert._isEqual(Object.prototype.hasOwnProperty.call(dependency, 'beep'), true)
199+
200+
// phase 2: reset uncovers the inherited getter again
201+
td.reset()
202+
assert._isEqual(Object.prototype.hasOwnProperty.call(dependency, 'beep'), false)
203+
assert._isEqual(dependency.beep(), 'og beep')
204+
},
205+
'with manual replacement' () {
206+
const fake = td.replace(dependency, 'beep', () => 'fake beep')
207+
208+
assert._isEqual(dependency.beep, fake)
209+
210+
// phase 2: reset uncovers the inherited getter again
211+
td.reset()
212+
assert._isEqual(Object.prototype.hasOwnProperty.call(dependency, 'beep'), false)
213+
assert._isEqual(dependency.beep(), 'og beep')
214+
}
215+
},
216+
'own and non-configurable (SWC exports)': {
217+
beforeEach () {
218+
Object.defineProperty(dependency, 'beep', {
219+
enumerable: true,
220+
get: () => () => 'og beep'
221+
})
222+
},
223+
'using automatic replacement' () {
224+
const ogDescriptor = Object.getOwnPropertyDescriptor(dependency, 'beep')
225+
226+
try {
227+
td.replace(dependency, 'beep')
228+
assert.fail('should error!')
229+
} catch (e) {
230+
assert._isEqual(e.message, 'Error: testdouble.js - td.replace - "beep" is defined with a getter that cannot be redefined. Compilers like SWC and Babel define ES module exports this way. To replace a module\'s export, replace the whole module with td.replace(\'path/to/module\') instead.')
231+
}
232+
assert._isEqual(Object.getOwnPropertyDescriptor(dependency, 'beep'), ogDescriptor)
233+
},
234+
'with manual replacement' () {
235+
const ogDescriptor = Object.getOwnPropertyDescriptor(dependency, 'beep')
236+
237+
try {
238+
td.replace(dependency, 'beep', () => 'fake beep')
239+
assert.fail('should error!')
240+
} catch (e) {
241+
assert._isEqual(e.message, 'Error: testdouble.js - td.replace - "beep" is defined with a getter that cannot be redefined. Compilers like SWC and Babel define ES module exports this way. To replace a module\'s export, replace the whole module with td.replace(\'path/to/module\') instead.')
242+
}
243+
assert._isEqual(Object.getOwnPropertyDescriptor(dependency, 'beep'), ogDescriptor)
244+
}
245+
},
246+
'with a setter': {
247+
beforeEach () {
248+
let beep = () => 'og beep'
249+
Object.defineProperty(dependency, 'beep', {
250+
configurable: true,
251+
enumerable: true,
252+
get: () => beep,
253+
set: (value) => { beep = value }
254+
})
255+
},
256+
'using automatic replacement' () {
257+
const ogDescriptor = Object.getOwnPropertyDescriptor(dependency, 'beep')
258+
259+
const double = td.replace(dependency, 'beep')
260+
261+
assert._isEqual(td.explain(double).isTestDouble, true)
262+
assert._isEqual(dependency.beep, double)
263+
assert._isEqual(Object.getOwnPropertyDescriptor(dependency, 'beep'), ogDescriptor)
264+
265+
// phase 2: reset restores through the setter
266+
td.reset()
267+
assert._isEqual(dependency.beep(), 'og beep')
268+
},
269+
'with manual replacement' () {
270+
const fake = td.replace(dependency, 'beep', () => 'fake beep')
271+
272+
assert._isEqual(dependency.beep, fake)
273+
274+
// phase 2: reset restores through the setter
275+
td.reset()
276+
assert._isEqual(dependency.beep(), 'og beep')
277+
}
278+
}
279+
},
152280
'Manually specifying the override': {
153281
beforeEach () {
154282
ogWarn = console.warn

0 commit comments

Comments
 (0)