thoughtbot / fishery

A library for setting up JavaScript objects as test data
MIT License
870 stars 36 forks source link

Question: how to have required params? #91

Closed romeerez closed 1 year ago

romeerez commented 2 years ago

Thank you for a very useful library!

My objects have onCreate and creating database records, there are required columns, but from TypeScript perspective all parameters are optional because of DeepPartial.

How to override factory params type to make some of the fields required?

stevehanson commented 2 years ago

Hi, thanks for reaching out. I'm afraid I don't fully understand the problem. Can you provide a simple/minimal code sample that illustrates the problem you are having?

All required fields for your object should be present on the object that is built with build(). The DeepPartial just means that you don't have to pass every field to build(). Any fields you leave out would be returned from your factory definition.

romeerez commented 2 years ago

Thanks for fast reply!

const record = MyRecordFactory.create() // compiles but doesn't work

The problem is that I need to remember which fields are required by this specific factory, so when using factory I need to jump to the source and read it and only then I can see that there is required userId or someOtherId:

const record = MyRecordFactory.create({ userId: 1, someOtherId: 2 }) // compiles and works

I mean that many tables have foreign keys and depends on other tables.

For build it is completely fine that none of the keys are required by factory, but for create I wish it was checked by TS

stevehanson commented 2 years ago

@romeerez can you share your factory? I am having trouble understanding:

My objects have onCreate and creating database records, there are required columns, but from TypeScript perspective all parameters are optional because of DeepPartial.

The resulting object you get from build or create will be typed according to your factory definition. Here is an example of what I understand you are trying to do:


type User = { name: string };
type SavedUser = User & { id: number };
const factory = Factory.define<User, {}, SavedUser>(({ onCreate }) => {
  // this function is typed as: (object: User) => SavedUser | Promise<SavedUser>
  onCreate(user => {
    // this should work, assuming this service takes a User and returns a SavedUser or Promise<SavedUser>
    return userService.save(user)

    // or, you can just manually add the ID if needed:
    return { ...user, id: 1 }

    // this won't compile because it is missing ID:
    return user
  });

  return { name: 'romeerez' };
});
thec0keman commented 2 years ago

I think I may be running into a similar use case. I'm typing the factory with what I want the final object to be, for example:

type Result = {
  id: number,
  caseId: number
}

At build time, I don't know what caseId should be, but if I don't provide a value, the type check will fail. I don't want to change the type, because that same type is used elsewhere, such as a component that will consume this object. So instead, I default to something like caseId: 0. However, what would be nice is when the factory is invoked, if the caller is required to pass caseId: number in order to pass the type check.

I'm not sure if this is something that can be solved purely in TypeScript, but a more manual solution would be adding a afterBuild that would verify caseId is set and > 0, and then log an error or throw.

What would be nice is if fishery had a mechanism to flag certain attributes as required params.

stevehanson commented 2 years ago

Thanks for sharing your situation @thec0keman. I've noted the feature request and will consider it for the future, though it's not something I've currently prioritized over other items on the roadmap.

In the meantime, while perhaps not ideal, you might consider creating your own build function to use instead of build:

class UserFactory extends Factory<User> {
 customBuild(params: DeepPartial<User> & { caseId: string }) {
    return this.build(params);
  }
}

const userFactory = UserFactory.define(() => {
  return {
    caseId: '1',
    firstName: 'Jack'
});

const user = userFactory.customBuild({ firstName: 'John' }); // type error, caseId not defined
klondikemarlen commented 6 months ago

I see that this is closed, but I've also run into this as well. Looking node_modules/fishery/dist/factory.d.ts I'd have to override almost every method to make this a native feature.

That said I built in a lesser thing that works using assertions. It doesn't trigger a type warning a definition time, but it will at least fail informatively. i.e.

import { faker } from "@faker-js/faker"
import { type DeepPartial } from "fishery"

import { DatasetField } from "@/models"
import { DatasetFieldDataTypes } from "@/models/dataset-field"

import BaseFactory from "@/factories/base-factory"

function assertParamsHasDatasetId(
  params: DeepPartial<DatasetField>
): asserts params is DeepPartial<DatasetField> & { datasetId: number } {
  if (typeof params.datasetId !== "number") {
    throw new Error("datasetId is must be a number")
  }
}

export const datasetFieldFactory = BaseFactory.define<DatasetField>(
  ({ sequence, params, onCreate }) => {
    onCreate((datasetField) => datasetField.save())

    assertParamsHasDatasetId(params)

    const description = faker.datatype.boolean(0.4) ? faker.lorem.sentence() : null
    const note = faker.datatype.boolean(0.1) ? faker.lorem.paragraph() : null

    return DatasetField.build({
      id: sequence,
      datasetId: params.datasetId,
      name: faker.database.column(),
      displayName: faker.database.column(),
      dataType: faker.helpers.enumValue(DatasetFieldDataTypes),
      description,
      note,
    })
  }
)

export default datasetFieldFactory
klondikemarlen commented 6 months ago

Hypothetically it might be possible to extend the "DeepPartial" function to accept non-partializing something when a specific brand is applied? e.g.

type RequiredBrand<T> = T & { _requiredBrand: unique symbol };

https://github.com/thoughtbot/fishery/blob/591a4b59278596fcfa8a36b1fe97975bbe2e320d/lib/types/deepPartial.ts#L19

export type DeepPartial<T> = T extends Primitive
  ...  
  ? T // Return these types unchanged
  : T extends object
  ? { [K in keyof T]?: T[K] extends RequiredBrand<infer U> ? U : DeepPartial<T[K]> }
  ...

Usage

type PartiallyPartialFoo = DeepPartial<SomeModel & { someAttribute: RequiredBrand<number> }>;

Where PartiallyPartialFoo would be partial, except for "someAttribute".