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

randSequence with charsType pattern #258

Merged
merged 3 commits into from
May 7, 2022
Merged

Conversation

pumano
Copy link
Contributor

@pumano pumano commented May 6, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[X] Other... Please describe:

fix flaky test check for random-float

What is the current behavior?

Issue Number: N/A

What is the new behavior?

randSequence can generate sequence via predefined char patterns:

  • alpha [Aa-Zz]
  • numeric [0-9]
  • alphaNumeric [Aa-Zz0-9]

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

@@ -43,11 +41,33 @@ type RandomSequenceOptions = {
*
* @example
*
* randSequence({ charType: 'numeric' }) // numeric | alpha | alphaNumeric (can't be used with 'chars' parameter )
Copy link
Member

Choose a reason for hiding this comment

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

We can remove (can't be used with 'chars' parameter ) and enforce by using type overloading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I can remove comment, but charType only works if chars not specified. If chars specified, chars got highest priority.

Copy link
Member

Choose a reason for hiding this comment

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

If using both doesn't make sense we can enforce it using typescript

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If using both doesn't make sense we can enforce it using typescript

OK, if I understand you right, I can create another charType with name custom and enforce user to specify chars, otherwise throw error, when chars specified without charType custom but it's breaking change I think

Copy link
Member

Choose a reason for hiding this comment

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

type RandomSequenceOptions = {
  size?: number;
  chars?: string;
}

type RandomSequenceOptions2 = {
  size?: number;
  charType?: 'numeric' | 'alpha' | 'alphaNumeric';
}

function foo(options: RandomSequenceOptions): any;
function foo(options: RandomSequenceOptions2): any;
function foo(options: any) {}

foo({charType: 'alphaNumeric', chars: ''}) // error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@NetanelBasal OK, I prepare commit soon

Copy link
Contributor Author

@pumano pumano May 6, 2022

Choose a reason for hiding this comment

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

@NetanelBasal looks like if I use that code, I really get typescript error, when trying to use wrong method signature, but I lost types because types returned using internal Return<string, Options>, can I expose that Return type via export?

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