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.

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

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

Merged
merged 5 commits into from Sep 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
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
76 changes: 76 additions & 0 deletions packages/jest-mock/__typetests__/mock-functions.test.ts
Expand Up @@ -383,3 +383,79 @@ 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'));

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'));
25 changes: 23 additions & 2 deletions packages/jest-mock/__typetests__/utility-types.test.ts
Expand Up @@ -62,6 +62,19 @@ class IndexClass {
}
}

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 someObject = {
SomeClass,

Expand Down Expand Up @@ -118,30 +131,38 @@ expectNotAssignable<FunctionLike>(someObject);

// ConstructorKeys

declare const constructorKeys: ConstructorLikeKeys<SomeObject>;
declare const interfaceConstructorKeys: ConstructorLikeKeys<OptionalInterface>;
declare const objectConstructorKeys: ConstructorLikeKeys<SomeObject>;

expectType<'SomeClass'>(constructorKeys);
expectType<'constructorA' | 'constructorB'>(interfaceConstructorKeys);
expectType<'SomeClass'>(objectConstructorKeys);

// MethodKeys

declare const classMethods: MethodLikeKeys<SomeClass>;
declare const indexClassMethods: MethodLikeKeys<IndexClass>;
declare const interfaceMethods: MethodLikeKeys<OptionalInterface>;
declare const objectMethods: MethodLikeKeys<SomeObject>;
declare const indexObjectMethods: MethodLikeKeys<IndexObject>;

expectType<'methodA' | 'methodB'>(classMethods);
expectType<'methodA' | 'methodB'>(indexClassMethods);
expectType<'methodA' | 'methodB'>(interfaceMethods);
expectType<'methodA' | 'methodB' | 'methodC'>(objectMethods);
expectType<'methodA' | 'methodB' | 'methodC'>(indexObjectMethods);

// PropertyKeys

declare const classProperties: PropertyLikeKeys<SomeClass>;
declare const indexClassProperties: PropertyLikeKeys<IndexClass>;
declare const interfaceProperties: PropertyLikeKeys<OptionalInterface>;
declare const objectProperties: PropertyLikeKeys<SomeObject>;
declare const indexObjectProperties: PropertyLikeKeys<IndexObject>;

expectType<'propertyA' | 'propertyB' | 'propertyC'>(classProperties);
expectType<string | number>(indexClassProperties);
expectType<'propertyA' | 'propertyB' | 'propertyC' | 'propertyD'>(
interfaceProperties,
);
expectType<'propertyA' | 'propertyB' | 'someClassInstance'>(objectProperties);
expectType<string | number>(indexObjectProperties);
108 changes: 57 additions & 51 deletions packages/jest-mock/src/index.ts
Expand Up @@ -41,11 +41,11 @@ export type ClassLike = {new (...args: any): any};
export type FunctionLike = (...args: any) => any;

export type ConstructorLikeKeys<T> = keyof {
[K in keyof T as T[K] extends ClassLike ? K : never]: T[K];
[K in keyof T as Required<T>[K] extends ClassLike ? K : never]: T[K];
};

export type MethodLikeKeys<T> = keyof {
[K in keyof T as T[K] extends FunctionLike ? K : never]: T[K];
[K in keyof T as Required<T>[K] extends FunctionLike ? K : never]: T[K];
};

export type PropertyLikeKeys<T> = Exclude<
Expand Down 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<T>,
V extends Required<T>[K],
>(object: T, methodKey: K, accessType: 'get'): SpyInstance<() => V>;

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

spyOn<
T extends object,
K extends ConstructorLikeKeys<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<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.

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