Effect-TS / data

Custom built data types leveraged by the Effect ecosystem
https://effect.website
MIT License
184 stars 22 forks source link

feat: added Types module #461

Closed jessekelly881 closed 1 year ago

jessekelly881 commented 1 year ago

Adds a collection of types used across effect repos

changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: 2f1ece3f89e98dd654ff5aed61f56e4ab3271f9d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ------------ | ----- | | @effect/data | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

mikearnaldi commented 1 year ago

I see where you're coming from as objects with a tag are very frequent but I am a bit reluctant to add a module for the sake of two type helpers that are basically an alias of Extract / Exclude. Namely whenever we had to deal with tags it is to do different things, almost never a simple exclusion or similar so I guess to bring this forward we'd need a strong motivation with concrete examples of app code.

Alternatively: we do have a number of type helpers that really have no place and we've been discussing the creation of a Types module to put there commonly used types such as IsEqual / UnionToIntersection & co (cc @tim-smart / @gcanti)

jessekelly881 commented 1 year ago

I'm inclined to agree with you. My first thought was actually to create a Types module and I only went with a Tagged module because the types that I wanted to add were only specific to tagged objects. I think a Types module would be pretty useful.

gcanti commented 1 year ago

in my TODO list there's an old "move UnionToIntersection to some centralized location", +1 for a Types module

jessekelly881 commented 1 year ago

Converted to Types module.

gcanti commented 1 year ago

@jessekelly881 There's also type Simplify<A> in src/Data (used for example in /schema).

However I would change it to:

export type Simplify<A> = {
  [K in keyof A]: A[K]
} extends infer B ? B : never

which IME produces shorter types in the emitted declaration files

jessekelly881 commented 1 year ago

I didn't see that specific type-def (I'm probably missing just overlooking it). I added that version of Simplify though and moved Data/IsEqualTo -> Types.Equal.

gcanti commented 1 year ago

I didn't see that specific type-def

you mean Simplify? is here https://github.com/Effect-TS/data/blob/62d6dc25f02dc489ce98399ae661fabdd00833e0/src/Data.ts#L191 (I guess you are a few commits behind main)

jessekelly881 commented 1 year ago

I was indeed a few commits behind. Lol.

jessekelly881 commented 1 year ago

Are there any other types that you guys can think of that we should add to this?

tim-smart commented 1 year ago

From /io:

image
mikearnaldi commented 1 year ago

LGTM @tim-smart if you want to push MergeRecord we can merge this

jessekelly881 commented 1 year ago

I added it. I renamed it to Type.Extend after S.extend and (at least to me) that seems more clear. Any quarrels w/ that name instead?

tim-smart commented 1 year ago

I added it. I renamed it to Type.Extend after S.extend and (at least to me) that seems more clear. Any quarrels w/ that name instead?

I think if it is called Extend, the second type parameter should have priority when it comes to key conflicts.

jessekelly881 commented 1 year ago

Make sense(although personally I'm kind of inclined to think the opposite, that the first type takes priority :P). Do you think it would be better to change the functionality or the name then? Maybe we have something like MergeRight(right priority), MergeLeft(left priority).

tim-smart commented 1 year ago

I think MergeRight and MergeLeft is good.

tim-smart commented 1 year ago

@jessekelly881 if you can run pnpm run docs and then we can merge this :)