Discuss boundaries of type-less JavaScript support about faker HOT 10 CLOSED

• A significant proportion of developers who are consumers of Faker do not use Typescript, or only use it some of the time
  Say blog.jetbrains.com/webstorm/2024/02/js-and-ts-trends-2024

And as your linked survey shows, 50% use VS Code, which supports TypeScript/JavaScript with type hints and can warn you about bad code, if you let it. Maybe <5% of JS developers use a type-less IDE.

• Creating a good DX (developer experience) for consumers of the library should outweigh making our code as simple as possible

Yes, but you only achieve that by consistency.
And that is exactly my point: checking for !value is a very rough check, that can accidentally be bypassed by previously valid calls.
IMO it is more likely that you bypass the check by accident, then actually running into it.

e.g. faker.date.between(123456, 456789) would pass that check, but would still result in invalid/unexpected values.
(it would implicitly fallback to new Date(undefined) => now for both from and to and thus return exactly that)

We could check for typeof options === 'object' && options != null && !options instanceof Date, but that would certainly be overkill (at least if we add this permanently).
When I get an error from a method, then I usually have a look at the documentation/signature to check what I might have done wrong.

• One of the key reasons to use a library like Faker rather than doing everything yourself is that it has a nice DX - everything is well documented, if you pass parameters incorrectly (e.g. reverse a min and a max value, forget something required) you get a helpful error message, etc.

I agree with that, for everything that typescript doesn't cover. (e.g. because of business logic or inter-variable constraints).

• We don't need to recreate all the checks that Typescript handles at compile time at runtime
• But, we shouldn't refuse to implement some helpful checks which are only needed in Javascript just because it requires some extra code

So which one should we add? If we cannot find a rule for that, we will talk about this again in the next PR.
In the case of faker.date.betweens, we have 4 lines of actual code. And 11 lines for value checks/throwing errors.

• Given that nearly all Faker methods do something sensible when called with no parameters, throw a helpful error if that's not the case for a specific method

What is so special about not providing any parameters? What about providing wrong/incompatible parameters?
IMO it is more likely to write working code first, that then breaks or the invocation context changes and thus no longer works.
So when you first implement something, you generally pay more attention to which methods you call, so you would normally not pass nothing if it requires something. But later you might put different values in your variable and it gets passed to the method by accident.

faker.date.between is a somewhat special case, that it previously worked without args and now requires some, so I think we could temporarily give it a specific and strict check, but we should eventually let that rely on TS inherent protections.

• Where a parameter takes a string enum value which could be easily mis-typed, do something sensible with unknown cases
  people passing really weird/unexpected values (e.g. passing a function to something that should be a string)

Once again, what is so special about it having a typo? Sure typos are easy, but what about passing a completely invalid value, like a number? or not quoting the value and thus using an undefined, unintended or uninitialized variable? Or having a typo in the name of the variable you intend to pass? AFAICT you don't get any error indications in your type-less code for that either.
IMO the line between "unexpected" and "really unexpected" values, is very diffuse if you look at the code needed to deal with it.

My main concern about adding a catch "other"/default clause is, that it also reduces our own DX as it hides issues like non-exhaustive-switch-case statements.

• Where we have evidence (Github issue reports etc) that people are confused by a cryptic error message in JS, add better errors

Which we have very few of. I could only find this one right now:

• #2503

And like that PR I would rather try to fix the underlying issue, than adding another check, that should never fail in the first place.
(e.g. because the missing locale data error is thrown first)

If we get such reports, then we can discuss whether and how we could improve the checks (or just add the missing data), but we shouldn't do so prematurely.

• Where code that would have been valid in earlier versions of Faker is deprecated or removed, and would lead to a cryptic error message for JS users

For a migration period, maybe yes, and if so it has to be a proper check, not a leaky wall.
Those checks not be there permanently.

• If it's a line or two of code to help out JS users
  where we'd have to write lots of code to handle JS users

How much of a method should be checks? Even if you add one or two lines at a time, you still end up with lots of checks.
Just have a look at between, it consists of 50% checks at best and 70% checks at worst.

from faker.

My feelings are

• A significant proportion of developers who are consumers of Faker do not use Typescript, or only use it some of the time
• Creating a good DX (developer experience) for consumers of the library should outweigh making our code as simple as possible
• One of the key reasons to use a library like Faker rather than doing everything yourself is that it has a nice DX - everything is well documented, if you pass parameters incorrectly (e.g. reverse a min and a max value, forget something required) you get a helpful error message, etc.
• We don't need to recreate all the checks that Typescript handles at compile time at runtime
• But, we shouldn't refuse to implement some helpful checks which are only needed in Javascript just because it requires some extra code

So generally I would support for example trying to handle these kind of cases in JS:

• Given that nearly all Faker methods do something sensible when called with no parameters, throw a helpful error if that's not the case for a specific method
• Where a parameter takes a string enum value which could be easily mis-typed, do something sensible with unknown cases
• Where we have evidence (Github issue reports etc) that people are confused by a cryptic error message in JS, add better errors
• Where code that would have been valid in earlier versions of Faker is deprecated or removed, and would lead to a cryptic error message for JS users
• If it's a line or two of code to help out JS users

But I don't think we need to add special JS code to handle

• every parameter and every method
• people passing really weird/unexpected values (e.g. passing a function to something that should be a string)
• where we'd have to write lots of code to handle JS users

from faker.

@faker-js/maintainers @faker-js/members @community Please share your thoughts on this topic.

from faker.

• A significant proportion of developers who are consumers of Faker do not use Typescript, or only use it some of the time

Do you have a source for this?

from faker.

Say https://blog.jetbrains.com/webstorm/2024/02/js-and-ts-trends-2024/

Or if you look at https://www.npmjs.com/browse/depended/@faker-js/faker there's a fair mix of JS and TS.

from faker.

Say blog.jetbrains.com/webstorm/2024/02/js-and-ts-trends-2024

Or if you look at npmjs.com/browse/depended/@faker-js/faker there's a fair mix of JS and TS.

Both of these give not any insight about TYPE-LESS JavaScript. Just because someone uses JS, doesn't mean they get no type support from JSDocs or an @ts-check comment.

from faker.

I'm not anti-types. I just think "this is not needed in TS" is insufficient justification for removing code which is useful for JS developers, e.g. which gives JS developers more useful error messages.

I would expect the vast majority of lines in a project like Faker to be documentation, error checking etc. At its core, a lot of Faker methods are basically thin wrappers around faker.helpers.arrayElement(faker.definitions.foo.bar). All the extra stuff we add on top of that is just to make developers' lives easier.

Anyway, don't feel you have to rebut all my points. if the consensus of the maintainers is different, I'll happily go along with that.

from faker.

Maybe <5% of JS developers use a type-less IDE.

That seems very low, I feel the developer ecosystem is way more diverse than that, and bear in mind developers are not always in an IDE. For example maybe i'm in a browser console. Or in the Node REPL. Or I've SSHed into a server and need to write a quick script to dump some fake data into my DB and I'm just using vi.

from faker.

Maybe <5% of JS developers use a type-less IDE.

That seems very low,

That is from your statistic. At least if we assume type capable IDEs.

from faker.

Team decision

We will not validate:

• types that are incompatible with the typescript type system, due to breaking changes in our api
  • an expection to this rule will be made if a change is introduced to a signature (WITHOUT prior deprection) which would fail for JS users with a native JS error
  • for these expections, we will throw an error for the entire major version the change was introduced
  • these errors are thrown instead of a simple deprecation, as we already include migration guides and breaking change lists on our major versions

We will validate:

• values which are compatible with the typescript type system, but might be invalid in a given context
  • example: max being smaller than min is contextually wrong and should be validated

These rules will be included in the upcoming maintainer docs.

from faker.