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

ts2.1: rearrange type translation, use new ObjectFlags #311

Merged
merged 1 commit into from
Jan 10, 2017
Merged

Conversation

evmar
Copy link
Contributor

@evmar evmar commented Jan 9, 2017

Make the main "translate" function match the enum in the
TypeScript API. Split out the handling of more complex
Object types into a separate function, and adjust the code
there to use the new objectFlags field.

This actually makes most tests pass(!) but there are still
a few more to track down, and there are still a bunch of
TODOs.

More work on #295.

@evmar
Copy link
Contributor Author

evmar commented Jan 9, 2017

Note: it's still intentional that the tests don't pass.

case ts.TypeFlags.EnumLiteral:
return 'number';
case ts.TypeFlags.ESSymbol:
// NOTE: currently this is just a typedef for {?}, shrug.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

case ts.TypeFlags.NumberLiteral:
return 'number';
case ts.TypeFlags.Never:
this.warn(`should not emit a 'never' type`);
Copy link
Contributor

Choose a reason for hiding this comment

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

why warning here? I think there are valid programs with the never type. Like users that mistrust the type system and want to augment it with a runtime check

switch (x)
 default:
  throw Error(x as any) <- x could be never here 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The question is what we want to do when we hit one of these. E.g. if you accidentally make an array of never, what type do we emit for Closure?

These warnings are never shown to a user and only show up inside the test suite, so it's probably harmless to leave it in... (?)

Copy link
Contributor

Choose a reason for hiding this comment

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

So it is for inexpressible concepts in Closure. Makes sense, maybe StringLiterals and etc *Literals should also get a warning then? No need to take care in the cl.

let isTuple = true;
// For unknown reasons, tuple types can be reference types containing a
// reference loop. see Destructuring3 in functions.ts.
// TODO(rado): handle tuples in their own branch.
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for taking care of my todo :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not done yet! There's an empty spot where tuples need to be handled. :P

Make the main "translate" function match the enum in the
TypeScript API.  Split out the handling of more complex
Object types into a separate function, and adjust the code
there to use the new objectFlags field.

This actually makes most tests pass(!) but there are still
a few more to track down, and there are still a bunch of
TODOs.

More work on #295.
@evmar evmar merged commit 276344b into ts2.1 Jan 10, 2017
@evmar evmar deleted the ts2.1-types branch January 10, 2017 00:20
rkirov pushed a commit that referenced this pull request Jan 13, 2017
Make the main "translate" function match the enum in the
TypeScript API.  Split out the handling of more complex
Object types into a separate function, and adjust the code
there to use the new objectFlags field.

This actually makes most tests pass(!) but there are still
a few more to track down, and there are still a bunch of
TODOs.

More work on #295.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants