feat: start can now watch ts files (#246) #276
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mcollina
requested changes
Oct 9, 2020
lib/watch/tsc-watcher.js
Outdated
| const appFile = resolve(configFile.compilerOptions.outDir, basename(opts._[0]).replace(/.ts$/, '.js')) | ||
| args.splice(args.length - 1, 1, appFile) | ||
|
|
||
| host.afterProgramCreate = async program => { |
There was a problem hiding this comment.
remaining of experiments
fixed now
test/start-typescript.test.js
Outdated
|
|
||
| // FIXME | ||
| // paths are relative to the root of the project | ||
| // this can be run only from there |
There was a problem hiding this comment.
what does this mean? Can you create an issue or fix it here?
There was a problem hiding this comment.
this is present in start.test.js too, sure, I'll fix it.
test/start-typescript.test.js
Outdated
| const test = t.test | ||
| const sgetOriginal = require('simple-get').concat | ||
| const sget = (opts, cb) => { | ||
| return new Promise((resolve, reject) => { |
There was a problem hiding this comment.
I used promisify.custom because, I think, it is not possible to simply use promisify if the callback has more than one param plus the err, in this case (err, response, body).
Am I wrong?
start.js
Outdated
| @@ -12,14 +12,16 @@ const pump = require('pump') | |||
| const isDocker = require('is-docker') | |||
| const listenAddressDocker = '0.0.0.0' | |||
| const watch = require('./lib/watch') | |||
| const tsWatch = require('./lib/watch/tsc-watcher') | |||
There was a problem hiding this comment.
can you load this only on demand? We are loading the entirety of typescript even if it's not a ts project.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
npm run testandnpm run benchmarkand the Code of conduct
The idea is that we should make the user run
fastify start...on .ts files and watch for changes on those files.At the moment the
npm run devscript in ts project template usesconcurrentlyso we basically have 2 watchers running together: one for ts files and one for the compileddistfiles.This is not ideal.
This approach uses only the tsc API to watch for .ts files changes. Every time it detects a change it kills fastify process, re-compiles changed files, restarts fastify.
Note: for the moment I assumed that if the target file is a .ts file we always want to watch it even without
-woption.Note: [docs](https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-
Any suggestion is well accepted.