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

Add “Composing types” #60

Merged
merged 3 commits into from
Mar 14, 2016
Merged

Add “Composing types” #60

merged 3 commits into from
Mar 14, 2016

Conversation

tomek-he-him
Copy link
Collaborator

Fixes #9.

This is also my contribution to the discussion on interface collisions. I think it should be valid to override more generic types like String with more specific ones like /^[a-z]+$/.

character: 'friendly' | 'grumpy',
}

// This will never be satisfied:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about "This is an rtype error: Interface type collision:"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, for the user it would be perfect if we detected type collisions statically. But such a promise might be risky on our side as there’s no obvious way to detect some conflicts. Like a: /^[A-Z]+$/ and a: /^[a-z]+$/ or n: (number) => number < 0 vs n: (number) => number > 10.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think mismatched literals are conflicts by definition, don't you? If they're the same type, they should be the same literal, shouldn't they?

We shouldn't try to infer compatible types for literal definitions. Right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that sounds reasonable and definitely makes things simpler. Fair enough. I’ll push an update ASAP.

(Though /^\w/ and /^[abc]/ are compatible. I can think of lots of other examples.)

@ericelliott
Copy link
Owner

Great work so far!

@ericelliott
Copy link
Owner

(Though /^\w/ and /^[abc]/ are compatible. I can think of lots of other examples.)

So can I, but is there an easy way to detect compatible types like this safely and automatically, for all compatible cases?

Also note, this is compatible one way, meaning all matches of /^[abc]/ will also match /^\w/, but the reverse is not true.

@tomek-he-him
Copy link
Collaborator Author

So can I, but is there an easy way to detect compatible types like this safely and automatically, for all compatible cases?

Nope, that’s what I meant too. Your idea is very good!

@tomek-he-him
Copy link
Collaborator Author

Does that sound better?

@tomek-he-him tomek-he-him force-pushed the tomekwi-merging-types branch from 65646fb to 99aa1d2 Compare January 23, 2016 08:37
@tomek-he-him
Copy link
Collaborator Author

Also, I’m wondering about one more implication of this syntax. I have the following in a project:

interface Integer
  (number) => (
    typeof number === 'number' &&
    number === parseInt(number, 10)
  );

interface PositiveInteger
  (number) => (
    typeof number === 'number' &&
    number === parseInt(number, 10) &&
    number > 0
  );

Since we can merge types by spreading them out within an interface block, should this mean the same?

interface Integer
  (number) => (
    typeof number === 'number' &&
    number === parseInt(number, 10)
  );

interface Positive
  (number) => (
    typeof number === 'number' &&
    number > 0
  );

interface PositiveInteger { ...Integer, ...Positive }

@ericelliott
Copy link
Owner

this mean the same?

interface Integer
  (number) => (
    typeof number === 'number' &&
    number === parseInt(number, 10)
  );

interface Positive
  (number) => (
    typeof number === 'number' &&
    number > 0
  );

interface PositiveInteger { ...Integer, ...Positive }

Yeah, I like this! =)

@ericelliott
Copy link
Owner

Are we adding an example like the one above?

@tomek-he-him
Copy link
Collaborator Author

Hi @ericelliott, I’ve been busy for a while – sorry.

One thing bothers me though. In the same section we say

To make sure we can run a static type check for you, we don’t allow merging two different literals. So this would result in a compile error:

interface Human {
  name: /^(.* )?[A-Z][a-z]+$/,
}

// Invalid!
interface Professor {
  ...Human,
  name: /^prof\. \w+$/,
}

Merging predicate literals like Integer and Positive is in fact the same thing. It can’t be statically checked.

How about making the static check opt-out? Some things (like merging literals) could just trigger a “you’re on your own” warning.

@ericelliott
Copy link
Owner

I want to see this in the spec. I think we can sidestep the issue for now by making it an error if you try to merge literals.

@ericelliott ericelliott mentioned this pull request Mar 14, 2016
@tomek-he-him
Copy link
Collaborator Author

Alright! I don’t think there were any other open issues. Should I merge?

@ericelliott
Copy link
Owner

Obviously, resolve the conflicts first.. =)

@tomek-he-him tomek-he-him merged commit 99aa1d2 into master Mar 14, 2016
@tomek-he-him tomek-he-him deleted the tomekwi-merging-types branch March 14, 2016 20:20
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