In the case of an object, you always pass comparisonKeys right?
If so, let's make this type stricter, and the values should also be a sub-set of T's keys.

If i'm honest I was having a little trouble with the types for this. I had a lot of issues when I tried to make this not optional. I have changed the comparisonKeys type from string[] to (keyof T)[] though

Why any and not a generic?

Yeah I wasn't a fan of this either. On further inspection i've managed to change it to objectIsUnique<T extends Record<string, any>>. I think this should be ok for now

Same

Same

I agree with Netanel's previous comment about the naming.
Maybe something like:
it(Should pass when object ids are unique) instead of the describe. This goes to all the specs I've seen in the recent files.
@NetanelBasal thoughts?

Agree

You are handling the length options both here and in the fake function, please choose one place

I think it makes sense to pass the attempts as the function's argument.

@shaharkazaz - How do you mean? To allow the user to decide the total number of attempts?

You are passing 0 to that factory function, I think it makes more sense to pass the attempts, if the factory is using the index passed than you will increase the chances of getting the same result.
@NetanelBasal WDYT?

Agree

same comment as above

There is a hidden assumption here that the array you are given holds unique values.
Please change the code to support non-unique arrays.

@shaharkazaz - You are correct. How did I not spot that :/

I have reworked it to shuffle the data array and then pick the first n items from it. A positive of doing it this way is that the user will receive an array with a length as close as possible to their desired length. The issue is that this will be more expensive for large data sets.

export function fakeFromArray<T, Options extends FakeOptions>(
  data: T[],
  options?: Options
): T | T[] {
  if (options?.length === 0) {
    return [];
  }

  if (!options?.length) {
    return randElement(data);
  }

  const priority = options?.priority ?? 'length';

  if (priority === 'length') {
    return Array.from({ length: options.length }, () => randElement(data));
  }

  let clonedData: T[] = JSON.parse(JSON.stringify(data));
  // Randomise order of data
  clonedData = shuffleArray<T>(clonedData);

  if (options.length >= clonedData.length) {
    return clonedData;
  }

  return clonedData.slice(0, length);
}

function shuffleArray<T>(array: T[]): T[] {
  for (let i = array.length - 1; i > 0; i--) {
    const randomIndex = Math.floor(random() * (i + 1));

    [
      array[i],
      array[randomIndex],
    ] = [
      array[randomIndex],
      array[i],
    ];
  }

  return array;
}

What are your thoughts?

You have a lot of shared code here, I think I would normalize the array data source to also work on a function with a random element and just use factory-based fake for both.

I'm happy to do this I just felt like this whole fake logic was getting quite complex so keeping them separate made it easier to read for me. But i'm happy either way.

I think in hindsight, if we were starting over i'd suggest that instead of option.length property we have singular and plural function for each. E.G randId that returns a string & randIds that uses randId to generate & return a string[]. But it might be a bit late for that ;) haha

Personally I prefer the length property, adding a plural will double the API for not a good enough reason from my point of view

Fair play. It works pretty well for our users, it just adds a lot of headaches for deving haha. All good though

This is repreated several times, maybe create a factory function that creates the chacker

function uniqueByKeys(keys){
  return (a, b) => objectIsUnique(keys)
}

@shaharkazaz - Sorry I don't know how I missed this. Do you mean something like:

export function randCreditCard<Options extends CreditCardOptions = never>(
  options?: Options
) {
  const factory: () => CreditCard = () => {
    const fullName =
      options?.fullName ??
      `${randPersonTitle()} ${randFullName({ withAccents: false })}`;
    const brand = options?.brand ?? (randCreditCardBrand() as Brand);

    const dateOptions: Intl.DateTimeFormatOptions = {
      month: 'numeric',
      year: '2-digit',
    };
    const validFrom = randPastDate({ years: 1 }).toLocaleDateString(
      'en-GB',
      dateOptions
    );
    const untilEnd = randFutureDate({ years: 2 }).toLocaleDateString(
      'en-GB',
      dateOptions
    );

    return {
      fullName,
      brand,
      validFrom,
      untilEnd,
      ccv: randCreditCardCVV(),
      number: randCreditCardNumber({ brand }),
      account: randAccount(),
      type: rand(['Credit', 'Debit']),
    };
  };

  return fake(
    factory,
    options,
    {
      // Moved logic into here rather than wrapping objectIsUnique call in new function
      uniqueComparer: (card, cards) => objectIsUnique(card, cards, ['number']) 
    }
  );
}

Isn't this checking the opposite? Don't you need all jsons to be different?
Looking at this as a reference:

Same comment as the json unique