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

Add typescript support #248

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion bin/express-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,9 @@ function createApplication (name, dir) {
pkg.devDependencies['typescript'] = '~3.7.5'
pkg.devDependencies['@types/node'] = '~13.7.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this cause an issue when generated and used on an older node.js like, say 10.x?

Copy link
Author

Choose a reason for hiding this comment

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

It should be safe on older node environment as the version of the generated codes are controlled via tsconfig.json (line 5, "target": "es5"). Currently it is set compiling codes down to ES5 to support most of the older node.js.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha. I guess i mean typescript-wise. As in I assume that the node 13 types include definitions that do not apply/work on node 10, which then reduces the useful type checking that would prevent that?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, you are correct.

As the typing version of node is not one-to-one, perhaps we can adjust it to match the major version of current node.

However, during testing I found that node typing below v8 causes typing mismatch errors so that I suggest setting the minimum of @types/node to v8.

// When below v8 (tested with v6.17.1):

node_modules/@types/connect/index.d.ts:21:42 - error TS2689: Cannot extend an interface 'http.IncomingMessage'. Did you mean 'implements'?

21     export class IncomingMessage extends http.IncomingMessage {
pkg.devDependencies['@types/node'] = '^' + Math.max(8, parseInt(process.version.replace('v', '')))
// example output => "@types/node": "^12"

pkg.devDependencies['@types/debug'] = '~4.1.5'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the types package version pach the debug package version?

pkg.devDependencies['@types/express'] = '~4.17.2'
pkg.devDependencies['@types/express'] = '~4.16.1'

pkg.dependencies['tslib'] = '~1.10.0'
Copy link
Author

Choose a reason for hiding this comment

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

As current settings automatically add the helper codes (e.g. ES6+ polyfills) into every related output files, I added tslib so that code duplication can be avoided.

}

// JavaScript
Expand Down
2 changes: 1 addition & 1 deletion templates/ts/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
// "tsBuildInfoFile": "./", /* Specify file to store incremental compilation information */
// "removeComments": true, /* Do not emit comments to output. */
// "noEmit": true, /* Do not emit outputs. */
// "importHelpers": true, /* Import emit helpers from 'tslib'. */
"importHelpers": true, /* Import emit helpers from 'tslib'. */
// "downlevelIteration": true, /* Provide full support for iterables in 'for-of', spread, and destructuring when targeting 'ES5' or 'ES3'. */
// "isolatedModules": true, /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */

Expand Down