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

@type support is a hack #91

Closed
domoritz opened this issue Jan 31, 2017 · 8 comments
Closed

@type support is a hack #91

domoritz opened this issue Jan 31, 2017 · 8 comments

Comments

@domoritz
Copy link
Collaborator

I don't like this patch. It works around the compiler with a hack. I prefer if we roll this back and ask people to use @TJS-type instead.

Comments @otgerrogla?

@domoritz
Copy link
Collaborator Author

I've files an issue with Typescript a while back microsoft/TypeScript#13498. I'd much prefer a fix in the compiler.

@otgerrogla
Copy link
Collaborator

Yes, it was meant as a temporary hack to keep backwards compatibility.

In fact I'm worried about conflicts by using @type, as it was meant for defining a different type for the schema than the one declared in Typescript, but now Typescript uses it internally which might lead to some issues. I think we should rather drop support for @type completely.

@domoritz
Copy link
Collaborator Author

I'd be fine with that. It is important to be able to say that a number is actually an integer or a string is a special type such as color but everything beyond this can break many things.

Okay, how about we drop support for @type and do a best effort to support @TJS-type? To do this, we rollback the change and make sure to document how to annotate in comments.

@otgerrogla
Copy link
Collaborator

Ok, I rolled back the commit.

@domoritz
Copy link
Collaborator Author

Thanks! I'll add something to the readme. can you look into #90?

@domoritz
Copy link
Collaborator Author

@otgerrogla I've added notes to the readme. In the future, let's not include the built js and js.map in the the repo until we actually make a new release.

@nidi3
Copy link

nidi3 commented Feb 1, 2017

Thanks for the readme. I was not aware of TJS-type annotation.

@domoritz
Copy link
Collaborator Author

domoritz commented Feb 1, 2017

@nidi3 You're welcome. We haven't been very good about documenting all the features until now but I hope things are clearer now.

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