-
-
Notifications
You must be signed in to change notification settings - Fork 943
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(helpers)!: stricter checking for function signature passed to multiple
#2563
Conversation
I'm not sure if this fits the use case of If you require the parameters of the mal function, you are most likely doing some static value generation. I need some time to think about this in more detail. |
Yes, usecase here is generating two sets of objects with matching ids, so I can "join" them later.
I could create an function with no arguments that simulates this behavior, but it seemed busywork since underlying code already supports it. I am using it in place without shared state (in mock service worker in two different endpoints) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index parameter might be useful when generating a list of entries that should have sequential ids.
However I would like to block this until v9.0 development commences, because then the method might get a new parameter (ref fakerCore) anyway.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2563 +/- ##
==========================================
- Coverage 99.57% 99.56% -0.02%
==========================================
Files 2846 2846
Lines 248715 248717 +2
Branches 655 1011 +356
==========================================
- Hits 247669 247634 -35
- Misses 1017 1083 +66
+ Partials 29 0 -29
|
Sorry for the delay in feedback. |
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
No problem, sorry for mine delay. Updated. |
Could you please fix the broken tests and examples?
|
Please also enable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub still asking me for review :)
In my opinion this change should be accepted and marked as breaking change.
In most cases this change should not lead to breaks, but it worth a notice.
method
parameter of multiple
multiple
Co-authored-by: Matt Mayer <152770+matthewmayer@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good migration guide 👍
faker.helpers.multiple() supports using mapFn like in Array.from(), but type signature of function didn't match, PR fixes this and adds test for this case
(but since it changes declared interface I chose
feat
instead offix
as semantic type)