-
-
Notifications
You must be signed in to change notification settings - Fork 550
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
base: master
Are you sure you want to change the base?
Conversation
Clone js template to ts template folder
Add TypeScript related dependencies and files
Update README
Remove unused dependency
|
||
pkg.devDependencies['typescript'] = '~3.7.5' | ||
pkg.devDependencies['@types/node'] = '~13.7.0' | ||
pkg.devDependencies['@types/debug'] = '~4.1.5' |
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.
Should the types package version pach the debug package version?
bin/express-cli.js
Outdated
pkg.devDependencies['typescript'] = '~3.7.5' | ||
pkg.devDependencies['@types/node'] = '~13.7.0' | ||
pkg.devDependencies['@types/debug'] = '~4.1.5' | ||
pkg.devDependencies['@types/express'] = '~4.17.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.
Should the types package version pach the express package version?
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.
Agree, better align with express v4.16.1.
Per check, @types/express also support typing on v4.16.
pkg.scripts['build'] = 'tsc' | ||
|
||
pkg.devDependencies['typescript'] = '~3.7.5' | ||
pkg.devDependencies['@types/node'] = '~13.7.0' |
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.
Would this cause an issue when generated and used on an older node.js like, say 10.x?
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.
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.
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.
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?
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.
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"
var wwwScriptName | ||
if (program.typescript) { | ||
appScriptName = 'app.ts' | ||
wwwScriptName = 'bin/www.ts' |
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.
The www
script shouldn't have an extension; it follows the pattern of being an executable (with shebang on the top).
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.
Got it, may be adding a post-build script to create a symbolic link with name www, pointing to www .js?
As some people may want to generate source-maps for the compiled JS files, I afraid directly renaming it may break the file linkage.
For example,
// add postbuild.js
var fs = require('fs')
var path = require('path')
// Create a symlink, linking www to www.js
var wwwScriptPath = path.join(__dirname, 'bin/www.js')
var wwwPath = path.join(__dirname, 'bin/www')
if (fs.existsSync(wwwScriptPath) && !fs.existsSync(wwwPath)) {
fs.symlinkSync(wwwScriptPath, wwwPath)
}
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.
Unfortunately normal users cannot create symbolic links on windows.
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.
How about if coping www .js as www?
var fs = require('fs')
var path = require('path')
// Polyfill copyFileSync for node < 8.5.0
if (!fs.copyFileSync) {
fs.copyFileSync = function (src, dest, flags) {
fs.writeFileSync(dest, fs.readFileSync(src), flags)
}
}
// Copy www.js to www
var wwwScriptPath = path.join(__dirname, 'bin/www.js')
var wwwPath = path.join(__dirname, 'bin/www')
if (fs.existsSync(wwwScriptPath)) {
fs.copyFileSync(wwwScriptPath, wwwPath)
}
test/cmd.js
Outdated
@@ -74,7 +74,8 @@ describe('express(1)', function () { | |||
' "http-errors": "~1.6.3",\n' + | |||
' "jade": "~1.11.0",\n' + | |||
' "morgan": "~1.9.1"\n' + | |||
' }\n' + | |||
' },\n' + | |||
' "devDependencies": {}\n' + |
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.
It is probably not add this if there are none; npm
will end up removing it on the next npm I
command a user uses. That's why we sort them in the same way npm does as well, so it doesn't cause churn in the file after the fact.
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.
Got it, will remove it and only add devDependencies on TypeScript templates.
pkg.devDependencies = {}
pkg.devDependencies['typescript'] = '~3.7.5'
// ...rest of codes
Awesome, I love this idea! I added a few questions/comments above. Note that I'm not familiar with TypeScript, so take them with a gain of salt :) It would be great if a test could be added as well. |
Updated to add devDependencies into package.json only when using TypeScript template
Adjust @types/express to align with express Include tslib to avoid duplicated polyfill codes in output
pkg.devDependencies['@types/express'] = '~4.17.2' | ||
pkg.devDependencies['@types/express'] = '~4.16.1' | ||
|
||
pkg.dependencies['tslib'] = '~1.10.0' |
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.
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.
Hi @dougwilson , |
Fixed the devDependecies to include the correct typings
When will this be released? I can't stand using plain javascript |
import debug from 'debug'; | ||
import * as http from 'http'; | ||
|
||
debug('<%- name %>:server'); |
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.
Doesn't look like the correct use of debug
. See documentation and/or the corresponding js code. I think this should be
import debugLogger from 'debug';
const debug = debugLogger('<%- name %>:server');
Same here, looks like nothing been done since 4 years |
I want to add TypeScript language support in the generator so that developers can enjoy strong-typed programming and use ES6 features in their applications.
Related issue: #125