thoughtbot / fishery

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

Build params mutate nested objects #88

Closed luchsamapparat closed 2 years ago

luchsamapparat commented 2 years ago

Description

In my project I use a couple of factories for different types to define a coherent set of data. For each type, I create a list of instances which may be referenced by other types:

const userFactory = Factory.define<User>(() => ({
    id: datatype.uuid(),
    firstName: name.firstName(),
    lastName: name.lastName(),
}));

const users = userFactory.buildList(20); // 👈 generate 20 random users

const taskFactory = Factory.define<Task>(() => ({
    title: lorem.words(),
    completed: datatype.boolean(),
    user: random.arrayElement(users) // 👈 randomly select one of the 20 users
}));

Sometimes, I also need to create variants of the same task:

const task = taskFactory.build();
const completedTask = taskFactory.build({ ...task, completed: true });

(In this simplified use-case it would be of course easier to just skip the factory and just write const completedTask = { ...task, completed: true }, but let's assume, that in real-world projects there might be good reasons to stick with the factory.)

That code seems to be harmless, but it actually mutated other users within my list of users which took the better part of my day to realize :)

From what I understand, this is what happens:

It might be up for debate if this is really a bug. But that behavior leads to side-effects that might be unintended and are hard to debug.

To Reproduce

Code Sandbox: https://codesandbox.io/s/fishery-test-forked-0rjz3?file=/src/index.test.ts

Alternatively: https://github.com/luchsamapparat/fishery-mutation-reproduction

Additional context

Modifying _mergeParamsOntoObject to merge into an empty object should solve the issue:

  _mergeParamsOntoObject(object: T) {
    merge({}, object, this.params, this.associations, mergeCustomizer);
  }
luchsamapparat commented 2 years ago

It seems that I ran into same problem again, but in another constellation. Already created objects were mutated when used in other factories. I could try to create another reproduction, but I guess the root cause is the same as described above.

Would you accept a PR for this?

stevehanson commented 2 years ago

@luchsamapparat thank you for the nudge. I just opened #89. Let me know if it resolves your issue!