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

feature request / idea: simplify types for fc.record #5434

Open
ahrjarrett opened this issue Nov 20, 2024 · 6 comments
Open

feature request / idea: simplify types for fc.record #5434

ahrjarrett opened this issue Nov 20, 2024 · 6 comments

Comments

@ahrjarrett
Copy link

ahrjarrett commented Nov 20, 2024

💡 Idea

Hi Nicolas -- first of all, thank you for your work on fast-check. It might be the only TS library I recommend to other devs without qualification :)

Anyway, in my own projects and at work, I usually add a package that re-exports fast-check, and includes various extensions that my team wants to use in more than one place.

One pattern I've found that helps seem to make the library "stick" a little better is to export a version of fc.record that ships a different set of types. I've included a version of it in the playground at the bottom of this issue if you'd like to play with it.

I wonder, would you accept a pull request that simplified the types for fc.record? I can submit that this weekend, if so, and would be happy to make any requested changes and help maintain them.

Motivation

The motivation is to make adoption of fast-check easier. One thing I've been experimenting with recently is using fast-check the way some devs use zod -- as the source of truth for my types.

For that to work, I think something like the change I'm proposing below is necessary, since otherwise the inferred types become quite difficult to read, especially when nested.

Example

Playground

@ahrjarrett ahrjarrett changed the title Record type feature request / idea: simplify types for fc.record Nov 20, 2024
@ahrjarrett
Copy link
Author

ahrjarrett commented Nov 20, 2024

Also I'm not sure which versions of TypeScript you've committed to supporting, so I created the playground in the oldest version that shipped TwoSlash annotations.

That was so you could more easily "see" the types, but I think this approach could work with older versions than 4.7.

One more thing, I'm realizing now that if a user constraints a record with { requiredKeys: [] }, that it might make more sense to make all the keys optional, rather than the other way around 🤔

@gruhn
Copy link
Contributor

gruhn commented Nov 21, 2024

Hey man, looks cool 🙂 I have to admit, both these and the original types take me a bit to digest. Why are all these utility types union-ed with never?

type Keep<T, K extends keyof T> = never | { [P in K]: T[P] }
//                                ^^^^^^^

@dubzzz
Copy link
Owner

dubzzz commented Nov 22, 2024

Thanks a lot for the suggestion. It could maybe be useful to rework the typings for record at some point. In the v4 (coming soon) I'm dropping some flags from it so that type definition is supposed to be simpler than today.

I have two objectives in mind for record:

  • Drop overloading definition - Overloading requires me to duplicate the JSDoc for each signature. It adds an extra load to the maintenance of the project, if I can drop it I will.
  • Make the typings even more powerful - Better typings for record in v4 #4570

@dubzzz
Copy link
Owner

dubzzz commented Nov 22, 2024

Said differently why not if it proves easier (I have not taken time to compare the current approach with your, I'll check later), but let's target the next major (don't want to break typings of existing users). regarding my two points above I think they are probably the target we should aim if we want to rewrite the type definition of record (2nd being more important than the overload given the value it would add).

@ahrjarrett
Copy link
Author

Hey man, looks cool 🙂 I have to admit, both these and the original types take me a bit to digest. Why are all these utility types union-ed with never?

type Keep<T, K extends keyof T> = never | { [P in K]: T[P] }
//                                ^^^^^^^

Hey @gruhn , I've been heads down on a project and didn't see your comment.

The union with never is an identity operation:

  • 0 | never === 0
  • ({ a: 0 } & { b: 1 }) | never === ({ a: 0 } & { b: 1 })
  • never | never === never, etc.

The dual operation is T & unknown, which is also an identity:

  • 1 & unknown === 1
  • (1 | 2) & unknown === 1 | 2
  • unknown & unknown === unknown, etc.

Seems kinda useless, but the reason is due to an implementation detail of the typechecker -- unifying or intersecting 2 types forces the typechecker to evaluate the pair, which forces evaluation.

If you've ever wished you could "see" the type that Omit<...> or Pick<...> represents, this is a nice way to do that.

@ahrjarrett
Copy link
Author

ahrjarrett commented Dec 3, 2024

Drop overloading definition - Overloading requires me to duplicate the JSDoc for each signature. It adds an extra load to the maintenance of the project, if I can drop it I will.

@dubzzz can you tell me more about what you mean here? Looking at the JSDocs for fc.record, if you drop all options* except requiredKeys for v4, you shouldn't need to add jsdocs to any overload except the first, and the rest will inherit from the first one.

I added that to the playground so you can see what I mean.

Also if you're pressed for time, I suggest skipping past the types and going straight to the usage at the bottom.

Edit: fixed a bug in the types.

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

No branches or pull requests

3 participants