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

Re-add simplified implementation of helpers.unique #2774

Open
ST-DDT opened this issue Mar 26, 2024 · 6 comments
Open

Re-add simplified implementation of helpers.unique #2774

ST-DDT opened this issue Mar 26, 2024 · 6 comments
Labels
c: feature Request for new feature m: helpers Something is referring to the helpers module p: 1-normal Nothing urgent s: waiting for user interest Waiting for more users interested in this feature
Milestone

Comments

@ST-DDT
Copy link
Member

ST-DDT commented Mar 26, 2024

Clear and concise description of the problem

The removal/deprecation of unique has an overwhelmingly negative feedback.

  • 25👎
  • 0👍

The other packages that provide the unique feature outside of faker got 12 hearts.

Suggested solution

Re-add a simplified implementation of the unique method, that requires providing dedicated a unique store option.

function unique(generator: (Faker) => T, options: { uniqueStore: T[] | Set<T> | (T) => boolean, maxAttempts?: number }) {
  const derived = faker.derive();
  for (i...maxAttempts) {
    const value = generator(derived);
    if (uniqueStore(value)) {
      return value
    }
  }
}

TBD: If there should be a default uniqueStore, potentially per faker instance.

Alternative

Keep using const values = helpers.uniqueArray and increment the offset manually.

Additional context

@ST-DDT ST-DDT added c: feature Request for new feature p: 1-normal Nothing urgent m: helpers Something is referring to the helpers module s: waiting for user interest Waiting for more users interested in this feature labels Mar 26, 2024
@ST-DDT ST-DDT added this to the v9.0 milestone Mar 26, 2024
Copy link
Contributor

Thank you for your feature proposal.

We marked it as "waiting for user interest" for now to gather some feedback from our community:

  • If you would like to see this feature be implemented, please react to the description with an up-vote (:+1:).
  • If you have a suggestion or want to point out some special cases that need to be considered, please leave a comment, so we are aware about them.

We would also like to hear about other community members' use cases for the feature to give us a better understanding of their potential implicit or explicit requirements.

We will start the implementation based on:

  • the number of votes (:+1:) and comments
  • the relevance for the ecosystem
  • availability of alternatives and workarounds
  • and the complexity of the requested feature

We do this because:

  • There are plenty of languages/countries out there and we would like to ensure that every method can cover all or almost all of them.
  • Every feature we add to faker has "costs" associated to it:
    • initial costs: design, implementation, reviews, documentation
    • running costs: awareness of the feature itself, more complex module structure, increased bundle size, more work during refactors

View more issues which are waiting for user interest

@Shinigami92
Copy link
Member

Please add to the description how the "faker-seed" is used inside that re-implementation.

@ST-DDT
Copy link
Member Author

ST-DDT commented Mar 26, 2024

@matthewmayer
Copy link
Contributor

I think the reason the removal issue got so many downvotes is because it was linked from the deprecation message without showing any alternative ways of generating unique values. So my feeling is that improving the documentation is more important than rushing to restore this.

@ST-DDT ST-DDT added the s: on hold Blocked by something or frozen to avoid conflicts label Mar 28, 2024
@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Apr 29, 2024

Upon reflection, the initial reason for removing the "unique" feature was its lack of dependency on Faker's seed. This remains a valid point, IMO. However, it has come to my attention that we currently have a function within our codebase, namely mergeLocales. The difference here is that the function is not provided under the Faker class scope., that operates independently of the Faker seed. This function is distinct in that it is not encapsulated within the Faker class.

In light of this, it may be valid to consider expanding the src/util/ directory to include additional utility functions that facilitate interaction with Faker, yet are not seed-dependent. I do not remember what the original intent for this directory was, tho.


A complete alternative might be to create a new package under our name space: @faker-js/util? Although, I wouldn't know how this would interact with things discussed in #704.

@klondikemarlen
Copy link

klondikemarlen commented May 13, 2024

Would it be possible to add the unique helper as a wrapper around other methods? Or as an option in each factory?

Given that I usually use a sequence to get locally unique values when using "fishery", the code might look like:

`${faker.company.name()}-${sequence}`

e.g.

import { faker } from "@faker-js/faker"
import { DeepPartial, Factory } from "fishery"

import { Team } from "@/models"

function assertParamsHasOrganizationId(
  params: DeepPartial<Team>
): asserts params is DeepPartial<Team> & { organizationId: number } {
  if (typeof params.organizationId !== "number") {
    throw new Error("organizationId is must be a number")
  }
}

export const teamFactory = Factory.define<Team>(({ sequence, params, onCreate }) => {
  onCreate((team) => team.save())

  assertParamsHasOrganizationId(params)

  return Team.build({
    id: sequence,
    organizationId: params.organizationId,
    name: `${faker.company.name()}-${sequence}`,
  })
})

export default teamFactory

So if Faker where to implement something like this it could look like

const name = unique(faker.company.name())

or

const name = faker.company.name({ unique: true })

The unique method would simply append an increasing sequential value to anything that was generated.
Unique dates and such would be more complex, but I find I rarely need unique dates, and when I do, I usually want to specify them explicitly.

The advantage of the first option would be that all uniqueness code would be centralized; the disadvantage is that it would be complicated and need to handle each data type.

The advantage of the second option is that it would help keep the complexity low by handling uniqueness on a per-factory level; the disadvantage is that it would require adding a new option to all factories.

@ST-DDT ST-DDT modified the milestones: v9.0, vFuture Jun 6, 2024
@ST-DDT ST-DDT removed the s: on hold Blocked by something or frozen to avoid conflicts label Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature m: helpers Something is referring to the helpers module p: 1-normal Nothing urgent s: waiting for user interest Waiting for more users interested in this feature
Projects
None yet
Development

No branches or pull requests

5 participants