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

For issue #170 type script improvements #171

Merged
merged 2 commits into from Feb 12, 2016

Conversation

atsu85
Copy link
Contributor

@atsu85 atsu85 commented Oct 22, 2015

Please merge this pull request and run gulp build to update dist folder.

This pull request improves generated TypeScript declarations (see #170).

This pull request includes initial work from @stephenlautier (see commit stephenlautier@1ea917b and stephenlautier@7a5eb63) and my fixes/improvements mentioned in his pull request https://github.com/aurelia/validation/pull/162/files as well other my own improvements on that matter for other methods.

Ats Uiboupin added 2 commits October 22, 2015 23:22
…tter aurelia-validation.d.ts

This commit includes initial work from @stephenlautier and my fixes/improvements mentioned in his pull request https://github.com/aurelia/validation/pull/162/files as well other my own improvements on that matter for other methods.
@atsu85
Copy link
Contributor Author

atsu85 commented Oct 22, 2015

Forgot to mention, that I've signed the CLA.

@stephenlautier I hope it is OK for You, that I included Your commits (mentioned You and added references to Your original commits to avoid "taking the glory" from You). Could You please confirm that this is OK for You so this pull request could be merged without any legal problems?

atsu85 pushed a commit to atsu85/validation that referenced this pull request Oct 22, 2015
This includes 3 pull requests in addition to version 0.3.1:
1) aurelia#166 - aurelia#167 - TypeError: Cannot read property 'classList' of null
2) aurelia#168 - aurelia#169 - makes TWBootstrapViewStrategyBase reusable
3) aurelia#170 - aurelia#171 - Improve generated TypeScript declarations
@stephenlautier
Copy link

Noooess... my first contribution to this awesome project! Kidding :). Sure go ahead, thanks for the comments.

As I stated in some of my type defs I updated mainly what I was more confident about hence that's why I set some of them to any (to not break anyone else by using wrong types or anything).

Cheers 👍

atsu85 pushed a commit to atsu85/validation that referenced this pull request Oct 22, 2015
…le to download custom package with jspm from my repo).

This includes 3 pull requests in addition to version 0.3.1:
1) aurelia#166 - aurelia#167 - TypeError: Cannot read property 'classList' of null
2) aurelia#168 - aurelia#169 - makes TWBootstrapViewStrategyBase reusable
3) aurelia#170 - aurelia#171 - Improve generated TypeScript declarations
@atsu85
Copy link
Contributor Author

atsu85 commented Oct 22, 2015

thanks @stephenlautier for Your contribution and permission to include Your work in this pull request!

I updated mainly what I was more confident about

Actually I'm also doing just that (or looking up smth from the source if documentation doesn't say enough).

... that's why I set some of them to any

When writing real TypeScript I've found it really disturbing when I see any type and by accident find out that i can't pass anything as an argument - that way in TypeScript it is always better to avoid adding any and wait someone else to add correct type info unless any really is the best type annotation for that argument or method return type.
Unfortunately when using babel-dts-generator there is an error when You specify type as a function with parameters, but don't add parameter types - that is where i sometimes give up and just add any when I don't want to spend too much time to figure out actual parameter types. Luckily this problem doesn't affect TypeScript comiler that i'm using most of the time :)

@stephenlautier
Copy link

Ahh I understand your point. Personally it doesn't bother me much, especially even when contributing to definitely typed they use

"noImplicitAny": true,

So you always have to give it a type, even when you don't know it. Generally when I see any I never threat it as any but as not defined. They should add unknown instead :p to remove ambiguity between any or any as in unknown/not set :)

@plwalters plwalters merged commit 553ee8e into aurelia:master Feb 12, 2016
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.

None yet

3 participants