Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(jest-mock): improve spyOn typings to handle optional properties #13247

Merged
merged 5 commits into from
Sep 11, 2022
Merged

fix(jest-mock): improve spyOn typings to handle optional properties #13247

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b4e0d13
fix(jest-mock): SpyOn handles optional properties
mrazauskas Sep 11, 2022
9ad649b
add changelog entry
mrazauskas Sep 11, 2022
80e68aa
clean up
mrazauskas Sep 11, 2022
9f5afc3
move Required
mrazauskas Sep 11, 2022
c0612ef
more tests
mrazauskas Sep 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

### Fixes

- `[jest-mock]` Improve `spyOn` typings to handle optional properties ([#13247](https://github.com/facebook/jest/pull/13247))

### Chore & Maintenance

### Performance
Expand Down
78 changes: 78 additions & 0 deletions packages/jest-mock/__typetests__/mock-functions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,3 +383,81 @@ expectType<SpyInstance<(value: {a: string}) => void>>(
expectError(spyOn(indexSpiedObject, 'propertyA'));

expectError(spyOn(indexSpiedObject, 'notThere'));

// interface with optional properties

class SomeClass {
constructor(one: string, two?: boolean) {}

methodA() {
return true;
}
methodB(a: string, b?: number) {
return;
}
}

interface OptionalInterface {
constructorA?: (new (one: string) => SomeClass) | undefined;
constructorB: new (one: string, two: boolean) => SomeClass;

propertyA?: number | undefined;
propertyB?: number;
propertyC: number | undefined;
propertyD: string;

methodA?: ((a: boolean) => void) | undefined;
methodB: (b: string) => boolean;
}

const optionalSpiedObject = {} as OptionalInterface;

expectType<SpyInstance<(one: string) => SomeClass>>(
spyOn(optionalSpiedObject, 'constructorA'),
);
expectType<SpyInstance<(one: string, two: boolean) => SomeClass>>(
spyOn(optionalSpiedObject, 'constructorB'),
);

expectError(spyOn(optionalSpiedObject, 'constructorA', 'get'));
expectError(spyOn(optionalSpiedObject, 'constructorA', 'set'));

// x.mockReturnValue(undefined);

expectType<SpyInstance<(a: boolean) => void>>(
spyOn(optionalSpiedObject, 'methodA'),
);
expectType<SpyInstance<(b: string) => boolean>>(
spyOn(optionalSpiedObject, 'methodB'),
);

expectError(spyOn(optionalSpiedObject, 'methodA', 'get'));
expectError(spyOn(optionalSpiedObject, 'methodA', 'set'));

expectType<SpyInstance<() => number>>(
spyOn(optionalSpiedObject, 'propertyA', 'get'),
);
expectType<SpyInstance<(arg: number) => void>>(
spyOn(optionalSpiedObject, 'propertyA', 'set'),
);
expectType<SpyInstance<() => number>>(
spyOn(optionalSpiedObject, 'propertyB', 'get'),
);
expectType<SpyInstance<(arg: number) => void>>(
spyOn(optionalSpiedObject, 'propertyB', 'set'),
);
expectType<SpyInstance<() => number | undefined>>(
spyOn(optionalSpiedObject, 'propertyC', 'get'),
);
expectType<SpyInstance<(arg: number | undefined) => void>>(
spyOn(optionalSpiedObject, 'propertyC', 'set'),
);
expectType<SpyInstance<() => string>>(
spyOn(optionalSpiedObject, 'propertyD', 'get'),
);
expectType<SpyInstance<(arg: string) => void>>(
spyOn(optionalSpiedObject, 'propertyD', 'set'),
);

expectError(spyOn(optionalSpiedObject, 'propertyA'));
expectError(spyOn(optionalSpiedObject, 'propertyB'));
104 changes: 55 additions & 49 deletions packages/jest-mock/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,40 +1008,48 @@ export class ModuleMocker {
return fn;
}

spyOn<T extends object, M extends PropertyLikeKeys<T>>(
spyOn<
T extends object,
K extends PropertyLikeKeys<Required<T>>,
V extends Required<T>[K],
>(object: T, methodKey: K, accessType: 'get'): SpyInstance<() => V>;

spyOn<
T extends object,
K extends PropertyLikeKeys<Required<T>>,
V extends Required<T>[K],
>(object: T, methodKey: K, accessType: 'set'): SpyInstance<(arg: V) => void>;

spyOn<
T extends object,
K extends ConstructorLikeKeys<Required<T>>,
V extends Required<T>[K],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

V is just a helper to avoid SpyInstance<(...args: ConstructorParameters<Required<T>[K]>) => InstanceType<Required<T>[K]>>

>(
object: T,
methodName: M,
accessType: 'get',
): SpyInstance<() => T[M]>;

spyOn<T extends object, M extends PropertyLikeKeys<T>>(
object: T,
methodName: M,
accessType: 'set',
): SpyInstance<(arg: T[M]) => void>;

spyOn<T extends object, M extends ConstructorLikeKeys<T>>(
object: T,
methodName: M,
): T[M] extends ClassLike
? SpyInstance<(...args: ConstructorParameters<T[M]>) => InstanceType<T[M]>>
methodKey: K,
): V extends ClassLike
? SpyInstance<(...args: ConstructorParameters<V>) => InstanceType<V>>
: never;

spyOn<T extends object, M extends MethodLikeKeys<T>>(
spyOn<
T extends object,
K extends MethodLikeKeys<Required<T>>,
V extends Required<T>[K],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines with Required do all the magic, because they turn types like (() => void)) | undefined into () => void). Something what wasn’t a function, becomes one. This part was not working, otherwise logic is the same.

>(
object: T,
methodName: M,
): T[M] extends FunctionLike
? SpyInstance<(...args: Parameters<T[M]>) => ReturnType<T[M]>>
methodKey: K,
): V extends FunctionLike
? SpyInstance<(...args: Parameters<V>) => ReturnType<V>>
: never;

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
spyOn<T extends object, M extends PropertyLikeKeys<T>>(
spyOn<T extends object, K extends PropertyLikeKeys<T>>(
object: T,
methodName: M,
methodKey: K,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

methodKey and K are just renames for consistency. MethodLikeKeys, PropertyLikeKeys, methodKey, propertyKey, K – always keys. Easier to understand what is happening.

accessType?: 'get' | 'set',
) {
if (accessType) {
return this._spyOnProperty(object, methodName, accessType);
return this._spyOnProperty(object, methodKey, accessType);
}

if (typeof object !== 'object' && typeof object !== 'function') {
Expand All @@ -1050,13 +1058,13 @@ export class ModuleMocker {
);
}

const original = object[methodName];
const original = object[methodKey];

if (!this.isMockFunction(original)) {
if (typeof original !== 'function') {
throw new Error(
`Cannot spy the ${String(
methodName,
methodKey,
)} property because it is not a function; ${this._typeOf(
original,
)} given instead`,
Expand All @@ -1065,14 +1073,14 @@ export class ModuleMocker {

const isMethodOwner = Object.prototype.hasOwnProperty.call(
object,
methodName,
methodKey,
);

let descriptor = Object.getOwnPropertyDescriptor(object, methodName);
let descriptor = Object.getOwnPropertyDescriptor(object, methodKey);
let proto = Object.getPrototypeOf(object);

while (!descriptor && proto !== null) {
descriptor = Object.getOwnPropertyDescriptor(proto, methodName);
descriptor = Object.getOwnPropertyDescriptor(proto, methodKey);
proto = Object.getPrototypeOf(proto);
}

Expand All @@ -1082,34 +1090,34 @@ export class ModuleMocker {
const originalGet = descriptor.get;
mock = this._makeComponent({type: 'function'}, () => {
descriptor!.get = originalGet;
Object.defineProperty(object, methodName, descriptor!);
Object.defineProperty(object, methodKey, descriptor!);
});
descriptor.get = () => mock;
Object.defineProperty(object, methodName, descriptor);
Object.defineProperty(object, methodKey, descriptor);
} else {
mock = this._makeComponent({type: 'function'}, () => {
if (isMethodOwner) {
object[methodName] = original;
object[methodKey] = original;
} else {
delete object[methodName];
delete object[methodKey];
}
});
// @ts-expect-error overriding original method with a Mock
object[methodName] = mock;
object[methodKey] = mock;
}

mock.mockImplementation(function (this: unknown) {
return original.apply(this, arguments);
});
}

return object[methodName];
return object[methodKey];
}

private _spyOnProperty<T extends object, M extends PropertyLikeKeys<T>>(
private _spyOnProperty<T extends object, K extends PropertyLikeKeys<T>>(
obj: T,
propertyName: M,
accessType: 'get' | 'set' = 'get',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default is unnecessary, accessType is always passed:

https://github.com/facebook/jest/blob/052aa67f22834933d986c8fb2a373379408ae5ea/packages/jest-mock/src/index.ts#L1043-L1045

propertyKey: K,
accessType: 'get' | 'set',
): Mock<() => T> {
if (typeof obj !== 'object' && typeof obj !== 'function') {
throw new Error(
Expand All @@ -1119,36 +1127,34 @@ export class ModuleMocker {

if (!obj) {
throw new Error(
`spyOn could not find an object to spy upon for ${String(
propertyName,
)}`,
`spyOn could not find an object to spy upon for ${String(propertyKey)}`,
);
}

if (!propertyName) {
if (!propertyKey) {
throw new Error('No property name supplied');
}

let descriptor = Object.getOwnPropertyDescriptor(obj, propertyName);
let descriptor = Object.getOwnPropertyDescriptor(obj, propertyKey);
let proto = Object.getPrototypeOf(obj);

while (!descriptor && proto !== null) {
descriptor = Object.getOwnPropertyDescriptor(proto, propertyName);
descriptor = Object.getOwnPropertyDescriptor(proto, propertyKey);
proto = Object.getPrototypeOf(proto);
}

if (!descriptor) {
throw new Error(`${String(propertyName)} property does not exist`);
throw new Error(`${String(propertyKey)} property does not exist`);
}

if (!descriptor.configurable) {
throw new Error(`${String(propertyName)} is not declared configurable`);
throw new Error(`${String(propertyKey)} is not declared configurable`);
}

if (!descriptor[accessType]) {
throw new Error(
`Property ${String(
propertyName,
propertyKey,
)} does not have access type ${accessType}`,
);
}
Expand All @@ -1159,7 +1165,7 @@ export class ModuleMocker {
if (typeof original !== 'function') {
throw new Error(
`Cannot spy the ${String(
propertyName,
propertyKey,
)} property because it is not a function; ${this._typeOf(
original,
)} given instead`,
Expand All @@ -1169,7 +1175,7 @@ export class ModuleMocker {
descriptor[accessType] = this._makeComponent({type: 'function'}, () => {
// @ts-expect-error: mock is assignable
descriptor![accessType] = original;
Object.defineProperty(obj, propertyName, descriptor!);
Object.defineProperty(obj, propertyKey, descriptor!);
});

(descriptor[accessType] as Mock<() => T>).mockImplementation(function (
Expand All @@ -1180,7 +1186,7 @@ export class ModuleMocker {
});
}

Object.defineProperty(obj, propertyName, descriptor);
Object.defineProperty(obj, propertyKey, descriptor);
return descriptor[accessType] as Mock<() => T>;
}

Expand Down