Skip to content

[#356] Add support for TypeScript 2.1.4 #365

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

Merged
merged 12 commits into from
Dec 27, 2016

Conversation

kamranayub
Copy link
Contributor

@kamranayub kamranayub commented Dec 25, 2016

This PR adds support for TS 2.1.4. Replaces #356

TODO

  • Check and test output for excaliburjs/Excalibur API
  • Update ts-internal comment references

Notable Changes

  • Added VS Code debugging support for tests
  • ModifierFlags were separated from NodeFlags so I had to update all those references
  • Constants are treated differently in 2.1, as documented here

Right now, TypeDoc will output an "unknown" type for constant declarations because they are no longer marked as Intrinsic types, instead they are Literal types. I think to properly support constants, there might need to be a new ReflectionKind.Constant so that constants can be grouped separately from variables.

@@ -7,139 +7,14 @@
"preserveConstEnums": true,
"declaration": true,
"sourceMap": true,
"inlineSources": true,
Copy link
Member

Choose a reason for hiding this comment

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

Please don't remove this. I've explained it a few times now, this is needed for debugging when you install from NPM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, sorry, I can do that.

"include": [
"src/**/*.ts"
],
"exclude": [
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the empty exclude?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exclude has to be present for TS to compile it correctly, because the default exclude is the current working directory.

error TS18003: No inputs were found in config file 'E:/Development/Contrib/typedoc/tsconfig.json'. Specified 'include' paths were '["src/**/*.ts"]' and 'exclude' paths were '["./"]'.

Copy link
Member

Choose a reason for hiding this comment

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

That sounds like a bug for someone else then, that's not a normal compiler issue but something must be mutating our config incorrectly.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it's been reported TypeStrong/grunt-ts#381.

Copy link
Contributor Author

@kamranayub kamranayub Dec 27, 2016

Choose a reason for hiding this comment

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

What would your preference be? Should I leave it in? It doesn't seem to break anything. I have already enabled passThrough in the gruntfile.

@@ -0,0 +1,23 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Please don't check the .vscode files into this project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are helpful for contributors, otherwise contributors have to manually figure out how to set up the debugger (this can be especially difficult for TS-based projects to know what the correct parameters are). It would be preferable to leave them in, there's nothing specific to my workspace that's in the files.

Copy link
Member

Choose a reason for hiding this comment

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

The thing specific to your workspace is VS code. Should we check in configurations for other IDEs too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like to optimize my projects for a specific xplat IDE, which is VS Code for me--it provides a happy path for contributors. However, you are the maintainer so would you be OK adding the configuration to the CONTRIBUTING readme? That provides a copy/paste way for contributors who want to use VS Code at least.

@blakeembrey
Copy link
Member

This PR is good, but it would be easier broken into separate PRs for the breaking functionality. I'll merge once those changes are fixed, thanks @kamranayub for your help!

@kamranayub
Copy link
Contributor Author

This is what constants look like in 2.1:

image

It's acceptable for now I think until proper Constant support is added.

@kamranayub
Copy link
Contributor Author

@blakeembrey when you have time (no rush) I had responded to your feedback 👍

@blakeembrey blakeembrey merged commit 8ed8989 into TypeStrong:master Dec 27, 2016
@kamranayub kamranayub deleted the 356-update-ts-214 branch December 27, 2016 18:14
@nycdotnet
Copy link

Hi - I updated grunt-ts to support include now, so the empty exclude should no longer be needed. Would you mind checking out [email protected] ?

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