-
-
Notifications
You must be signed in to change notification settings - Fork 738
Updated TypeScript version into 2.4.1 #549
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
Conversation
set noStrictGenericChecks typescript compiler option to true
The fix implied more changes than expected:
|
CHANGED decorators.ts for bad property used CHANGED react.d.ts to solve a known issue
|
Finally, the build compiles and run the tests with
I need to do a lot of changes and fixes, so I think that I've done something wrong in some stage, could a core developer help me to fix this? Thanks I leave the pull request at this stage to have a changes track |
Any idea when this will get released? (This is working in my repo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aleixmorgadas First of all, thank you for your work and sorry I took so long to review. I know you had to go through a few iterations to get this working. I made a few inline comments but here is a summary.
tslint
I'm not sure why the tslint issues are coming up now but I'd prefer for them to be done in a separate PR. The whitespacing on imports isn't consistent either. Not sure how others feel about this.
tsconfig.json changes
The tsconfig does to define the typeRoots
because TypeScript handles the default well enough. The noStrictGenericChecks
flag is the simplest fix to the new compilation errors but I would like to fix the type checking.
TS 2.4 specific features
This isn't really the responsibility of this PR but I wanted to note that we should review how new features like string Enums and dynamic imports are handled.
package.json
Outdated
@@ -1,7 +1,7 @@ | |||
{ | |||
"name": "typedoc", | |||
"description": "Create api documentations for typescript projects.", | |||
"version": "0.7.1", | |||
"version": "0.7.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is normally updated on release. Also need to think about whether this is a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this can be undone, I'll do a release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 😁
src/index.ts
Outdated
export {Application} from './lib/application'; | ||
export {CliApplication} from './lib/cli'; | ||
export {Application } from './lib/application'; | ||
export {CliApplication } from './lib/cli'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why no space in front?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed the file, strange that tslint didn't notice it
tsconfig.json
Outdated
@@ -16,7 +16,10 @@ | |||
"outDir": "dist/", | |||
"rootDir": "src/", | |||
"experimentalDecorators": true, | |||
"noStrictGenericChecks": true | |||
"noStrictGenericChecks": true, | |||
"typeRoots": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be necessary. TypeScript defaults to this.
@@ -43,6 +43,6 @@ function decoratorWithParam(value:boolean):MethodDecorator { | |||
*/ | |||
function decoratorWithOptions(options:{name:string}):ClassDecorator { | |||
return function (target) { | |||
target.options = options; | |||
target.arguments = target; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So there error here is caused by a lib.es5.d.ts definition. Granted, this is only an example being documented but I'd prefer to update the definition of the target function to include this property.
The major changes made are to support TS 2.4 as soon as possible, and then remove the parches to apply the best practices. I'm OK with your decision to first apply the best procedure and then release the new version. So, I will make a new PR for the tslint issue in the next days. How do you want to procedure with the ts version upgrade? |
👍 |
We're really looking forward to the release of this! |
@aleixmorgadas Thanks for doing the whitespace changes in a separate PR. I made some changes to this PR to tighten the generics type definitions and avoid some of the CI timeouts. @blakeembrey Can you take a quick look and create a release for this? The version number was already bumped. |
Hi, I also need this update, when will be merged? |
@CosminIonascu In the meantime you can install this pull request by running: |
Ok, thank's! |
Also @CosminIonascu, you will ned to build the dist folder in order be able to run the node script: |
tsconfig.json
Outdated
"es5", | ||
"es2015.collection", | ||
"es2015.iterable" | ||
"es2016" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
es2016
? Is there something from es2016
we're using or should this just be es2015
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there was some issues with the types due to the last typescript version, it was solved using the es2016 lib
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which issues?
@blakeembrey I've undone the change in the There was a lot of errors at the beginning, so they guided me to that change. View all thread to find the reason. |
@aleixmorgadas The changes you made were invalid JSON. I'll try to check it out for myself. Edit: Actually, it passes with no changes to the |
@blakeembrey Yes the commit d3d4d42 had an invalid JSON, but the first errors weren't. |
@aleixmorgadas As far as I can see, there's no changes to |
Oh, I see what you meant now too. I thought you were undoing the change to demonstrate it failing. The order of the conversation confused me, sorry. I saw your TravisCI link first and I thought that was you showing it fails using the |
set noStrictGenericChecks typescript compiler option to true due to Generic Error