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.

Sign up for GitHub

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

Feat/watching nonexistant dependencies #97

Closed
wants to merge 2 commits into from
Closed

Feat/watching nonexistant dependencies #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
29c0104
also watching files that don't currently exist
Sep 16, 2019
01a0ae9
execute permission on ts-node-dev
Oct 4, 2019
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
Empty file modified bin/ts-node-dev
100644 → 100755
View file
Open in desktop
Empty file.
14 changes: 13 additions & 1 deletion lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ var tsNodeVersion = require('ts-node').VERSION
var tsVersion = require('typescript').version
var kill = require('tree-kill')
var readline = require('readline')
var path = require('path')
var chokidar = require('chokidar')

module.exports = function(script, scriptArgs, nodeArgs, opts) {
if (typeof script !== 'string' || script.length === 0) {
Expand Down Expand Up @@ -38,6 +40,7 @@ module.exports = function(script, scriptArgs, nodeArgs, opts) {
// Run ./dedupe.js as preload script
if (cfg.dedupe) process.env.NODE_DEV_PRELOAD = __dirname + '/dedupe'

var chokidarWatches = []
var watcher = filewatcher({
forcePolling: opts.poll,
interval: parseInt(opts.interval),
Expand Down Expand Up @@ -150,7 +153,14 @@ module.exports = function(script, scriptArgs, nodeArgs, opts) {

// Upon errors, display a notification and tell the child to exit.
ipc.on(child, 'error', function(m) {
log.debug('Child error')
log.debug('Child error', m)
var lastRequire = m.lastRequire
if (m.code === 'MODULE_NOT_FOUND' && lastRequire && lastRequire.path.startsWith('.')) {
var pathNoExt = path.normalize(path.join(path.dirname(lastRequire.filename), lastRequire.path))
var watch = chokidar.watch([pathNoExt + '.ts', pathNoExt + '.tsx'])
Copy link

Choose a reason for hiding this comment

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

why not just call watcher.add() here? What does chokidar do that filewatcher() cant?

Copy link
Author

Choose a reason for hiding this comment

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

@jonjaques Please enlighten me if I missed something, but watcher.add() didn't work with non-existant files when I tried

Copy link

Choose a reason for hiding this comment

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

Naw, was legit curious.

As I understand it this PR is to handle the case where you're in an existing file and you import something that doesn't yet exist:

import {something} from './nonexistent'

This handles the error that would shut down ts-node-dev and then sets up a watcher for what would be ./nonexistent.ts|tsx.

I guess what I meant was, both filewatcher and chokidar are wrappers around the FS.watch, it just seems like conceptually this project should use one or the other.

So different question— could we switch all the filewatcher code to use chokidar instead? Last publish on filewatcher was 3 years ago.

Copy link
Author

Choose a reason for hiding this comment

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

I'm all for doing that @jonjaques! I think chokidar is more mature

However, I'm no official contributor. Could coordinate if @whitecolor made me

watch.on('all', (arg, file) => restart(file, false))
chokidarWatches.push(watch)
}
notify(m.error, m.message, 'error')
stop(m.willTerminate)
})
Expand Down Expand Up @@ -199,6 +209,8 @@ module.exports = function(script, scriptArgs, nodeArgs, opts) {
return
}
log.debug('Removing all watchers from files')
chokidarWatches.forEach(nw => nw.close())
chokidarWatches = []
watcher.removeAll()
starting = true
if (child) {
Expand Down
19 changes: 16 additions & 3 deletions lib/wrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var Module = require('module')
var path = require('path');
var childProcess = require('child_process');
var fork = childProcess.fork;
Expand All @@ -21,6 +22,15 @@ if (process.env.NODE_DEV_PRELOAD) {
require(process.env.NODE_DEV_PRELOAD);
}

// We hook on the require call to keep track of the last required files in case there's a
// MODULE_NOT_FOUND error for file watching
var lastRequire = null;
var origRequire = Module.prototype.require;
Module.prototype.require = function (requirePath) {
lastRequire = { path: requirePath, filename: this.filename };
return origRequire.apply(this, arguments);
};

// Listen SIGTERM and exit unless there is another listener
process.on('SIGTERM', function () {
if (process.listeners('SIGTERM').length === 1) process.exit(0);
Expand All @@ -45,13 +55,16 @@ process.on('uncaughtException', function (err) {
// If there's a custom uncaughtException handler expect it to terminate
// the process.
var hasCustomHandler = process.listeners('uncaughtException').length > 1;
var isTsError = err.message && /TypeScript/.test(err.message)
if (!hasCustomHandler && !isTsError) {
var isTsError = err.message && /TypeScript/.test(err.message);
if (!hasCustomHandler && !isTsError) {
console.error(err.stack || err);
}
}

ipc.send({
error: isTsError ? '' : err.name || 'Error',
message: err.message,
code: err.code,
lastRequire: lastRequire,
willTerminate: hasCustomHandler
});
});
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@
"test-docker": "docker run --rm -v ${PWD}:/app mhart/alpine-node:8.7.0 sh -c 'cd app && node ./bin/ts-node-dev --cache-directory .ts-node test/ts/big'"
},
"dependencies": {
"chokidar": "^3.1.0",
"dateformat": "~1.0.4-1.2.3",
"dynamic-dedupe": "^0.3.0",
"filewatcher": "~3.0.0",
"minimist": "^1.1.3",
"mkdirp": "^0.5.1",
"node-notifier": "^5.4.0",
"resolve": "^1.0.0",
"rimraf": "^2.6.1",
"rimraf": "^2.7.1",
"source-map-support": "^0.5.12",
"tree-kill": "^1.2.1",
"ts-node": "*",
Expand Down