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

Factory produces data incompatible to its type constraint #106

Open
lo1tuma opened this issue Sep 22, 2022 · 2 comments
Open

Factory produces data incompatible to its type constraint #106

lo1tuma opened this issue Sep 22, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@lo1tuma
Copy link

lo1tuma commented Sep 22, 2022

Description

When using the factory overrides to set a property explicitly to undefined this is always allowed even when the type constraint doesn’t accept undefined as a valid value.

To Reproduce

import { Factory } from 'fishery';

interface Foo {
    bar: string;
}

const fooFactory = Factory.define<Foo>(() => {
    return { bar: 'any-value' };
});

const result = fooFactory.build({ bar: undefined }); // result has the type Foo but its value is { bar: undefined } which is not a valid Foo

Additional context

It seems like this bug has been introduced with #44

@lo1tuma lo1tuma added the bug Something isn't working label Sep 22, 2022
@stevehanson
Copy link
Contributor

Thank you for reporting this issue, and apologies for the delay in getting back. This does seem to be an issue. Ensuring that factories are type-safe is more important than allowing users to un-set a property, which is what the motivation behind #44. I am leaning right now toward reverting #44 and releasing it as a major version bump.

Currently, all params passed to build are optional, and TypeScript by default allows explicitly passing undefined for properties that are typed as optional (eg. bar?: string). Ideally, TypeScript would only allow omitting the property but not passing undefined, so that fooFactory.build({ bar: undefined }) would produce a type error. This behavior is only possible with a TypeScript compiler options flag, exactOptionalPropertyTypes, which most users will not have set.

@carmanchris31
Copy link

My team uses the factory to produce values for undefined properties and produce objects with no optional properties. Today I lost valuable time troubleshooting failing tests because a build function was returning an incomplete object due to this issue.

There isn't an obvious distinction between an explicitly undefined value and an implicit undefined from omission and IMO this is just too surprising and error-prone:

import {Factory} from 'fishery'

interface PointModel {
  x: number;
  y: number;
}

const pointFactory = Factory.define<PointModel>(() => {
  return {
    x: 0,
    y: 0
  }
})

const data: Partial<PointModel> = {x: 5}

const a = pointFactory.build(data)
console.log(a) // {x: 5, y: 0}
const b = pointFactory.build({ x: data.x, y: data.y })
console.log(b) // {x: 5, y: undefined}

https://playcode.io/1777612

I would be in favor of reverting with a major version bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants