8000 Add `IndexOf`, `LastIndexOf`, `IndicesOf`, `CountOf` types by benzaria · Pull Request #1155 · sindresorhus/type-fest · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add IndexOf, LastIndexOf, IndicesOf, CountOf types #1155

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

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

benzaria
Copy link
Contributor
@benzaria benzaria commented May 26, 2025

Equivelent to Array.prototype.[ indexOf | lastIndexOf ]

External

  • IndexOf Returns the index of the first occurrence of a value in an array, or -1 if it is not present.

  • LastIndexOf Returns the index of the last occurrence of a value in an array, or -1 if it is not present.

  • IndicesOf Return an array of indexes of all the occurrences of a value in an array, or [] if it is not present.

  • CountOf Return the count of all the occurrences of a value in an array, or 0 if it is not present.

  • Includes Modified to use IndexOf

…se` types

 **External**
 - `IndexOf` Returns the index of the first occurrence of a value in an array, or -1 if it is not present.
 - `LastIndexOf` Returns the index of the last occurrence of a value in an array, or -1 if it is not present.
 - `AllIndexOf` Return an array of indexes of all the occurrences of a value in an array, or [] if it is not present.
 - `CountOf` Return the count of all the occurrences of a value in an array, or 0 if it is not present.
 - `ArrayReverse` Reverse an array.
@benzaria
Copy link
Contributor Author
benzaria commented Jun 1, 2025

@som-sm Hi, can u review this Please. Thanks !

@sindresorhus
Copy link
Owner

Looks ok to me 👍

We cannot really review in detail without tests.


AllIndexOf => IndicesOf
ArrayReverse => Reverse

/**
Simpler version of Sum<T, 1>, without the extra logic.
*/
export type Increment<T extends number> = SumPositives<T, 1>;
Copy link
Owner

Choose a reason for hiding this comment

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

Why are you exporting it? It's not used elsewhere.

Copy link
Contributor Author
@benzaria benzaria Jun 2, 2025

Choose a reason for hiding this comment

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

Ohh i forgot about it i was using it in a arithmetic types I'm working on to Improve Sum, Substract with a range of 10999 number, with also Multiplication, Division. do u consider adding does types or it just to much ?

@som-sm
Copy link
Collaborator
som-sm commented Jun 3, 2025

For now there is no tests or docs until approval!

@benzaria If you want to understand the feasibility of a type, please consider opening an issue first. Because without proper tests it doesn't really make sense to review the PR.

Copy link
Collaborator
@som-sm som-sm left a comment

Choose a reason for hiding this comment

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

@benzaria A proper review would only be possible when tests are added. Also, every type needs proper documentation with examples.

@som-sm
Copy link
Collaborator
som-sm commented Jun 3, 2025

Also, please submit one type per PR, unless the types are closely related. Once you start handling various edge cases, the implementations can become quite verbose and complex, so separating them into individual PRs makes a lot of sense.

And even for simpler types, it's best to keep them in separate PRs whenever possible.

@benzaria benzaria changed the title Add IndexOf, LastIndexOf, AllIndexOf, CountOf and ArrayReverse types Add IndexOf, LastIndexOf, IndicesOf, CountOf and ArrayReverse types Jun 3, 2025
@benzaria benzaria changed the title Add IndexOf, LastIndexOf, IndicesOf, CountOf and ArrayReverse types Add IndexOf, LastIndexOf, IndicesOf, CountOf, Reverse, SplitOnSpread and ExtractStrict types Jun 4, 2025
@benzaria benzaria requested review from sindresorhus and som-sm June 4, 2025 18:21
Copy link
Collaborator
@som-sm som-sm left a comment

Choose a reason for hiding this comment

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

Also, please submit one type per PR, unless the types are closely related. Once you start handling various edge cases, the implementations can become quite verbose and complex, so separating them into individual PRs makes a lot of sense.

And even for simpler types, it's best to keep them in separate PRs whenever possible.

Doesn't make sense to add seven different types in one single PR. I'd rather prefer seven independent PRs.

Here’s what I’d suggest:
Pick one of the seven types, open a PR just for that, and get it reviewed and merged. Once you're familiar with the process and expectations, you can move on to opening PRs for the remaining types.

CC: @sindresorhus

@benzaria
Copy link
Contributor Author
benzaria commented Jun 4, 2025

@som-sm @sindresorhus Hey guys, I'm so sorry for the many types I made in this PRs. I know it's better to separate them in diff PRs. but when a type relies on the other it's not ideal take for example the Xor-XorAll which relay on this PRs to work and now both of them is pending approval. and Xor-XorAll can't pass tests unless the CountOf type is present.

Making a PRs for each individual type and wait to merge will take a lot of time, and will delay the contribution.

the SplitOnSpread was intentionally to be an internal type for making Reverse work, but I thought it can be useful for others. What u think guys shood we expose it ?

@benzaria benzaria changed the title Add IndexOf, LastIndexOf, IndicesOf, CountOf, Reverse, SplitOnSpread and ExtractStrict types Add IndexOf, LastIndexOf, IndicesOf, CountOf types Jun 5, 2025
@benzaria benzaria marked this pull request as draft June 5, 2025 11:09
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.

3 participants
0