Skip to content

Use spread operator to override values (fixes #114) #131

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

Closed
wants to merge 2 commits into from

Conversation

emab
Copy link
Contributor

@emab emab commented Apr 24, 2023

This PR implements a new way of generating mock types. Instead of relying on checking overrides for a specific property name, we instead just spread any provided overrides over the key:value object of field to value.

In my opinion this has a couple of benefits:

  • Easier to read. There's less going on per line - it's clear what the generated value is.
  • Eslint may encourage you to not use .hasOwnProperty
  • Don't need to use ! to satisfy TypeScript

I've updated all tests, and looking through the snapshots looks positive!

Copy link
Owner

@ardeois ardeois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but I the history a little and we actually did this implementation at first
Then we switched to hasOwnProperty to fix a recursive issue
#9

Not sure if this issue will cause a problem now we have relationshipsToOmit handling ... ?
What do you think?

@ardeois ardeois added the major Increment the major version when merged label Apr 25, 2023
@emab
Copy link
Contributor Author

emab commented May 2, 2023

Looks good, but I the history a little and we actually did this implementation at first Then we switched to hasOwnProperty to fix a recursive issue #9

Not sure if this issue will cause a problem now we have relationshipsToOmit handling ... ? What do you think?

Yeah that is a good point actually. This method will call the mock data function and then apply overrides. That mock function would then cause a recursive call as it would also call the previous function again, if we don't provide a way to terminate.

I think this would mean the config option to turn on protection for circular relationships would have to be enabled at all times, otherwise it would cause a breakage. I wonder how often people disable it?

@stale
Copy link

stale bot commented Jun 8, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 8, 2023
@stale stale bot closed this Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Increment the major version when merged stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants