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

Added integrity checking and HKTAs #158

Merged

Conversation

SimonMeskens
Copy link
Collaborator

Here's the POC for HKTAs. It's a very simple replacement for URI2HKT. It's just a bit more typesafe (forces use of HKTS) and stops you from using wrongly defined URI2HKT mappings. It also means users don't have to use indexers anymore, but just plain generics, to access the URI2HKT map.

@SimonMeskens
Copy link
Collaborator Author

I'm not quite sure why prettier fails btw, I'll need some help with that.

@gcnew
Copy link

gcnew commented Jul 18, 2017

While reading the code, I was wondering whether there is a specific reason to use:

export type HKTS = keyof {
  [key in URI2HKT<any>[keyof URI2HKT<any>]['_URI']]: any
}

instead of simply:

export type HKTS = URI2HKT<any>[keyof URI2HKT<any>]['_URI']

@gcnew
Copy link

gcnew commented Jul 18, 2017

This POC looks good from my perspective.

@SimonMeskens
Copy link
Collaborator Author

Yup, that simplication works, odd that I didn't think of that, I keep forgetting you can pass disjunct unions an indexer. You want to merge it in? It needs a fix for the "prettier" fail though, I'm not sure what's wrong

@gcnew
Copy link

gcnew commented Jul 18, 2017

How do you run it/how is it supposed to be run? I didn't see an npm run lint macro.

@gcanti
Copy link
Owner

gcanti commented Jul 18, 2017

you can run fix-prettier

@gcnew
Copy link

gcnew commented Jul 18, 2017

Oops, my bad. I was looking at the bottom of the file and didn't see that the scrips are actually at the top. Thank, you!

@gcanti
Copy link
Owner

gcanti commented Jul 19, 2017

In Identity.ts should be

declare module './HKT' {
-  interface URIHKT<A> {
+  interface URI2HKT<A> {
     Identity: Identity<A>
   }
 }

@gcanti
Copy link
Owner

gcanti commented Jul 19, 2017

It also means users don't have to use indexers anymore, but just plain generics, to access the URI2HKT map

Good idea.

@SimonMeskens I'd love to also keep that small 2-liner (...2 bugs found ;)

@gcanti
Copy link
Owner

gcanti commented Jul 19, 2017

Weird, just tried moving that 2-liner to tests but it doesn't work there

@SimonMeskens
Copy link
Collaborator Author

Yeah, I did the same thing and moved it to tests, but somehow that doesn't work. I think it has to do with imports and merged declarations. I'll put it in HKT.ts for now

@gcanti
Copy link
Owner

gcanti commented Jul 19, 2017

@SimonMeskens to fix travis, you can run npm run fix-prettier

@SimonMeskens
Copy link
Collaborator Author

Aha, that does seem to work, thanks, now I understand. Prettier just checks, fix-prettier to actually fix, makes sense

@gcanti
Copy link
Owner

gcanti commented Jul 19, 2017

(yeah, I use vscode with prettier onsave, so usually I don't run fix-prettier)

Excellent, LGTM

@SimonMeskens
Copy link
Collaborator Author

I'm going to let you handle merging. Do you want any rebases done?

@gcanti gcanti merged commit 3ff4c78 into gcanti:overloadings-destroyer Jul 20, 2017
@gcanti
Copy link
Owner

gcanti commented Jul 20, 2017

Thanks @SimonMeskens

@SimonMeskens SimonMeskens deleted the overloadings-destroyer branch July 21, 2017 16:26
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

Successfully merging this pull request may close these issues.

3 participants