thoughtbot/fishery

Subset of items for buildList should be build for each item

Opened this issue · 4 comments

Description

First of all, thank you guys for your hard work, this is really cool lib!
During the integration with the fishery, we found one behavior that seems wrong.
Imagine that we have Users with Posts and I what to generate a list of users with random posts, let's say 5 users with 5 posts.
To make it more useful I've done it like this:

type Post = {
  id: number;
}
type User = {
  id: number;
  posts: Post[];
};

const postFactory = Factory.define<Post>(({ sequence }) => ({ id: sequence }));

class UserFactory extends Factory<User> {
  withPosts(posts?: Post[]){
    return this.params({
      posts: posts || postFactory.buildList(5),
    })
  }
}


const userFactory = UserFactory.define(({ sequence }) => {
  return {
    id: sequence,
    posts: [],
  };
});

const users = userFactory.withPosts().buildList(5);

So after I created a list of users I expected to have 25 different posts, but I have 5 users with the same 5 posts.

To Reproduce

CodeSandbox with a failing test

Thank you for reporting this and for the simple example and CodeSandbox. They were both very helpful in understanding the problem you're facing.

This behavior is a consequence of the way builder functions, like withPosts in your example, are implemented. They are only run once at the time you call them and their returned params then are folded into the final object when build is called.

So, basically, what is happening is:

// returns a new userFactory that has "{ posts: [{id:1},{id:2},{id:3},{id:4},{id:5}] }" saved as "extra" params
const userFactoryWithPosts = userFactory.withPosts()

// To build the user, this takes the return object from the factory and then merges 
// the stored "extra" params and then merges the build params ("{ id: 1 }") to 
// construct the final object. Does not regenerate extra params
userFactoryWithPosts.build({ id: 1 });

This would be one way to get your desired behavior, though it's cumbersome (Sandbox):

const users = [
    userFactory.withPosts().build(),
    userFactory.withPosts().build(),
    userFactory.withPosts().build(),
    userFactory.withPosts().build(),
    userFactory.withPosts().build()
  ];

Ideally, we could instead run the builder functions dynamically at build time (i.e. instead of storing params in the factory, we could store an array of builder functions to call prior to build). I believe this would fix your issue. I don't have time to look into this more right now, but I'd be open to considering this behavior if someone else wanted to explore it.

No problem at all!
Thanks for the workaround, I've already achieved what I need.

If you can point me where I can help you with it, I can try to create PR for this. I found the place where buildList is declared but it seems like making such a dynamic build will take more significant changes.

Thanks in advance!

@vanatd-printify How were you able to achieve it? I hit the exact same problem and since I use a circularReplacer function when I JSON.stringify, it detects those objects as being the same and thus removes them.

Same problem