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

feat: pass in index to FactoryValueGenerator #43

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

simplenotezy
Copy link

@simplenotezy simplenotezy commented Aug 6, 2024

Description

This PR introduces a new feature in the FactoryValueGenerator class to include the current index in its operations. This enhancement allows for greater flexibility, e.g. the creation of consistent UUIDs across different seeds, which is particularly useful during development.

Key Benefits

  • Consistent UUID Generation: By incorporating the current index, the generated UUIDs can remain consistent across reseeds.
  • Improved Developer Productivity: Developers can then refire the same API requests without needing to reconfigure IDs, streamlining the development process.

Implementation Details

  • The FactoryValueGenerator has been updated to include the current index in its UUID generation logic.
  • No breaking changes are introduced; existing functionalities remain intact. I had to add ts-ignore though, which I am not particularly proud of, but seems to be the most sensible solution to maintain backwards compatability

Example

@Factory((_, __, index) =>
  uuidv5(index.toString(), '6ba7b810-9dad-11d1-80b4-00c04fd430c8'),
)
id: string;

Checklist

  • Code has been tested locally
  • Documentation has been updated
  • All tests pass N/A

Please review the changes and provide feedback. Thanks!

@simplenotezy simplenotezy changed the title Feature/add index param feat: pass in index to FactoryValueGenerator Aug 6, 2024
@simplenotezy simplenotezy force-pushed the feature/add_index_param branch from 764a1e2 to b1e924e Compare August 6, 2024 10:56
@edwardanthony
Copy link
Owner

Hey Mattias,

Thank you for submitting the PR. I'm considering whether injecting the index would be beneficial.

When creating IDs, it's generally best for the database or the driver to determine the sequence. This approach minimizes logic errors since the database acts as the single source of truth for ID resolution.

For example, in MongoDB, ObjectId handles this automatically. Similarly, SQL databases use features like autoincrement or bigserial to generate sequential IDs. Even in cases where IDs are created manually, such as Twitter's Snowflake ID, an index isn't necessary.

However, if you still need to access the index, you can easily achieve this with the following code:

const users = DataFactory.createForClass(User)
  .generate(10)
  .map((user, index) => ({ ...user, id: index }));

Feel free to let me know what you think.

@simplenotezy
Copy link
Author

Hey @edwardanthony - that's defiantly also an approach that could work, didn't think of that. This naturally wouldn't be an issue if you use auto incrementing IDs, but I have stopped using that years ago in favour of UUIDs which are a bit more discrete, espeically when it comes to revealing business metrics to the outside world.

While I have you; how do you go about seeding relationships? E.g. when using TypeORM.

I find my self having to first create the entities, and then map over each entity and assign relationships. Having the IDs prepared in advance, prior to saving them in the DB, also greatly helps this process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants