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

Support defining the type when extending a factory with params() #75

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 37 additions & 0 deletions lib/__tests__/factory-builders.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,11 @@ describe('associations', () => {
});

describe('params', () => {
interface AdminUser extends User {
admin: true;
adminPrivileges: string[];
}

it('adds parameters that are then used for build', () => {
const user = userFactory.params({ admin: true }).build();
expect(user.admin).toBe(true);
Expand All @@ -226,6 +231,38 @@ describe('params', () => {
expect(adminFactory.build().admin).toBe(true);
expect(adminFactory.build().admin).toBe(true);
});

it('adds parameters for a sub-type which are then also accepted in build()', () => {
const adminFactory = userFactory.params<AdminUser>({
admin: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

My concern is that if admin: true were not supplied, then the factory happily compiles but does not actually return an AdminUser.

adminPrivileges: ['any-privilege'],
});

expect(adminFactory.build()).toEqual({
admin: true,
adminId: null,
adminPrivileges: ['any-privilege'],
firstName: 'Yussef',
id: '1',
lastName: 'Sanchez',
memberId: null,
post: { id: '1' },
registered: false,
});
expect(
adminFactory.build({ adminPrivileges: ['changed-privilege'] }),
).toEqual({
admin: true,
adminId: null,
adminPrivileges: ['changed-privilege'],
firstName: 'Yussef',
id: '1',
lastName: 'Sanchez',
memberId: null,
post: { id: '1' },
registered: false,
});
});
});

describe('transient', () => {
Expand Down
6 changes: 4 additions & 2 deletions lib/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,12 @@ export class Factory<T, I = any> {
* @param params
* @returns a new factory
*/
params(params: DeepPartial<T>): this {
params<TOverride extends T = T>(
params: DeepPartial<TOverride>,
): Factory<TOverride, I> {
const factory = this.clone();
factory._params = merge({}, this._params, params, mergeCustomizer);
return factory;
return factory as unknown as Factory<TOverride, I>;
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue, and I'm not sure yet how to solve it, is that even though TOverride extends T, we aren't guaranteeing that new params are passed in such that the built object is actually a TOverride.

Copy link
Author

Choose a reason for hiding this comment

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

You are absolutely right, params is Partial and might not contain all properties required for TOverride. 🤔 I also can’t think about any other approach to make it work with the params() method. Maybe with a dedicated extends() method that accepts another generator function.

}

/**
Expand Down