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

Symbol properties inside objects passed to the build function as associations are not getting merged into result #105

Open
asla3 opened this issue Jul 8, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@asla3
Copy link

asla3 commented Jul 8, 2022

Description

If you call a factory's build function passing an object that contains symbols as properties, the symbols props won't be merged into the build result as seen here:

type User = {
  name: string;
};

type Post = {
  name: string;
  user: User;
};

const userFactory = Factory.define<User>(() => ({
  name: "Susan"
}));

const postFactory = Factory.define<Post>(() => ({
    name: "Post 1",
    user: userFactory.build()
  }));

const sym = Symbol("a");

const userWithSymbol = { ...userFactory.build(), [sym]: "a" };

const postFactoryResult = postFactory.build(
    {},
    { associations: { user: userWithSymbol } }
);

console.log(postFactoryResult.user[sym]) // it should return "a" but it's returning undefined instead because user doesn't have the symbol prop.

To Reproduce

I made this sandbox that showcases this behavior: https://codesandbox.io/s/fishery-test-forked-30eett?file=/src/index.test.ts

Additional context
After peaking around for a bit, I found out that this happens because the mergeCustomizer used in this library doesn't handle symbols, so it relies on lodash.merge behaviour which doesn't merge symbols by default as seen in this issue. For now, I found this workaround that does merge symbols into the resulting object:

const postFactory = Factory.define<Post>(({associations}) => ({
    name: "Post 1",
    user: associations.user || userFactory.build()
  }));

My guess is that it works because user gets the same object we passed that already has the symbols present on it, so when fishery starts merging our factory's return value with associations and params it doesn't remove the symbols that are already present on user.

@asla3 asla3 added the bug Something isn't working label Jul 8, 2022
@asla3
Copy link
Author

asla3 commented Jul 10, 2022

Just a quick update: When I opened this issue I was hopeful that this could be implemented by just slightly changing mergeCustomizer because in the issue I linked a mantainer tells the poster to try mergeWith and use a customizer with it that fits their use case. However, after playing with it for a bit, I discovered that the reason symbol props are not being merged is because lodash's merge doesn't iterate over these properties, only over string keyed ones. It mentions it on the docs but I completely missed it:

...it recursively merges own and inherited enumerable string keyed properties of source objects...

So to support symbol keyed properties on objects passed through associations it would be necessary to switch to another merge function that is not lodash's.

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

1 participant