diff --git a/src/Microsoft.AspNetCore.NodeServices/Configuration/Configuration.cs b/src/Microsoft.AspNetCore.NodeServices/Configuration/Configuration.cs index 4bda4d6..a497c64 100644 --- a/src/Microsoft.AspNetCore.NodeServices/Configuration/Configuration.cs +++ b/src/Microsoft.AspNetCore.NodeServices/Configuration/Configuration.cs @@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.NodeServices switch (options.HostingModel) { case NodeHostingModel.Http: - return new HttpNodeInstance(options.ProjectPath, /* port */ 0, options.WatchFileExtensions); + return new HttpNodeInstance(options.ProjectPath, options.WatchFileExtensions, /* port */ 0); case NodeHostingModel.Socket: var pipeName = "pni-" + Guid.NewGuid().ToString("D"); // Arbitrary non-clashing string return new SocketNodeInstance(options.ProjectPath, options.WatchFileExtensions, pipeName); diff --git a/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-http.js b/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-http.js index 9e665ab..ab728ae 100644 --- a/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-http.js +++ b/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-http.js @@ -57,14 +57,9 @@ var http = __webpack_require__(2); var path = __webpack_require__(3); var ArgsUtil_1 = __webpack_require__(4); - var AutoQuit_1 = __webpack_require__(5); // Webpack doesn't support dynamic requires for files not present at compile time, so grab a direct // reference to Node's runtime 'require' function. var dynamicRequire = eval('require'); - var parsedArgs = ArgsUtil_1.parseArgs(process.argv); - if (parsedArgs.watch) { - AutoQuit_1.autoQuitOnFileChange(process.cwd(), parsedArgs.watch.split(',')); - } var server = http.createServer(function (req, res) { readRequestBodyAsJson(req, function (bodyJson) { var hasSentResult = false; @@ -117,6 +112,7 @@ } }); }); + var parsedArgs = ArgsUtil_1.parseArgs(process.argv); var requestedPortOrZero = parsedArgs.port || 0; // 0 means 'let the OS decide' server.listen(requestedPortOrZero, 'localhost', function () { // Signal to HttpNodeHost which port it should make its HTTP connections on @@ -170,35 +166,5 @@ exports.parseArgs = parseArgs; -/***/ }, -/* 5 */ -/***/ function(module, exports, __webpack_require__) { - - "use strict"; - var fs = __webpack_require__(6); - var path = __webpack_require__(3); - function autoQuitOnFileChange(rootDir, extensions) { - // Note: This will only work on Windows/OS X, because the 'recursive' option isn't supported on Linux. - // Consider using a different watch mechanism (though ideally without forcing further NPM dependencies). - fs.watch(rootDir, { persistent: false, recursive: true }, function (event, filename) { - var ext = path.extname(filename); - if (extensions.indexOf(ext) >= 0) { - console.log('Restarting due to file change: ' + filename); - // Temporarily, the file-watching logic is in Node, so we signal it's time to restart by - // sending the following message back to .NET. Soon the file-watching logic will move over - // to the .NET side, and this whole file can be removed. - console.log('[Microsoft.AspNetCore.NodeServices:Restart]'); - } - }); - } - exports.autoQuitOnFileChange = autoQuitOnFileChange; - - -/***/ }, -/* 6 */ -/***/ function(module, exports) { - - module.exports = require("fs"); - /***/ } /******/ ]))); \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-socket.js b/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-socket.js index 3035b34..1585613 100644 --- a/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-socket.js +++ b/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-socket.js @@ -44,7 +44,7 @@ /* 0 */ /***/ function(module, exports, __webpack_require__) { - module.exports = __webpack_require__(7); + module.exports = __webpack_require__(5); /***/ }, @@ -83,54 +83,19 @@ /***/ }, /* 5 */ -/***/ function(module, exports, __webpack_require__) { - - "use strict"; - var fs = __webpack_require__(6); - var path = __webpack_require__(3); - function autoQuitOnFileChange(rootDir, extensions) { - // Note: This will only work on Windows/OS X, because the 'recursive' option isn't supported on Linux. - // Consider using a different watch mechanism (though ideally without forcing further NPM dependencies). - fs.watch(rootDir, { persistent: false, recursive: true }, function (event, filename) { - var ext = path.extname(filename); - if (extensions.indexOf(ext) >= 0) { - console.log('Restarting due to file change: ' + filename); - // Temporarily, the file-watching logic is in Node, so we signal it's time to restart by - // sending the following message back to .NET. Soon the file-watching logic will move over - // to the .NET side, and this whole file can be removed. - console.log('[Microsoft.AspNetCore.NodeServices:Restart]'); - } - }); - } - exports.autoQuitOnFileChange = autoQuitOnFileChange; - - -/***/ }, -/* 6 */ -/***/ function(module, exports) { - - module.exports = require("fs"); - -/***/ }, -/* 7 */ /***/ function(module, exports, __webpack_require__) { "use strict"; // Limit dependencies to core Node modules. This means the code in this file has to be very low-level and unattractive, // but simplifies things for the consumer of this module. - var net = __webpack_require__(8); + var net = __webpack_require__(6); var path = __webpack_require__(3); - var readline = __webpack_require__(9); + var readline = __webpack_require__(7); var ArgsUtil_1 = __webpack_require__(4); - var AutoQuit_1 = __webpack_require__(5); - var virtualConnectionServer = __webpack_require__(10); + var virtualConnectionServer = __webpack_require__(8); // Webpack doesn't support dynamic requires for files not present at compile time, so grab a direct // reference to Node's runtime 'require' function. var dynamicRequire = eval('require'); - var parsedArgs = ArgsUtil_1.parseArgs(process.argv); - if (parsedArgs.watch) { - AutoQuit_1.autoQuitOnFileChange(process.cwd(), parsedArgs.watch.split(',')); - } // Signal to the .NET side when we're ready to accept invocations var server = net.createServer().on('listening', function () { console.log('[Microsoft.AspNetCore.NodeServices:Listening]'); @@ -179,29 +144,30 @@ // Begin listening now. The underlying transport varies according to the runtime platform. // On Windows it's Named Pipes; on Linux/OSX it's Domain Sockets. var useWindowsNamedPipes = /^win/.test(process.platform); + var parsedArgs = ArgsUtil_1.parseArgs(process.argv); var listenAddress = (useWindowsNamedPipes ? '\\\\.\\pipe\\' : '/tmp/') + parsedArgs.listenAddress; server.listen(listenAddress); /***/ }, -/* 8 */ +/* 6 */ /***/ function(module, exports) { module.exports = require("net"); /***/ }, -/* 9 */ +/* 7 */ /***/ function(module, exports) { module.exports = require("readline"); /***/ }, -/* 10 */ +/* 8 */ /***/ function(module, exports, __webpack_require__) { "use strict"; - var events_1 = __webpack_require__(11); - var VirtualConnection_1 = __webpack_require__(12); + var events_1 = __webpack_require__(9); + var VirtualConnection_1 = __webpack_require__(10); // Keep this in sync with the equivalent constant in the .NET code. Both sides split up their transmissions into frames with this max length, // and both will reject longer frames. var MaxFrameBodyLength = 16 * 1024; @@ -382,13 +348,13 @@ /***/ }, -/* 11 */ +/* 9 */ /***/ function(module, exports) { module.exports = require("events"); /***/ }, -/* 12 */ +/* 10 */ /***/ function(module, exports, __webpack_require__) { "use strict"; @@ -397,7 +363,7 @@ function __() { this.constructor = d; } d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); }; - var stream_1 = __webpack_require__(13); + var stream_1 = __webpack_require__(11); /** * Represents a virtual connection. Multiple virtual connections may be multiplexed over a single physical socket connection. */ @@ -438,7 +404,7 @@ /***/ }, -/* 13 */ +/* 11 */ /***/ function(module, exports) { module.exports = require("stream"); diff --git a/src/Microsoft.AspNetCore.NodeServices/HostingModels/HttpNodeInstance.cs b/src/Microsoft.AspNetCore.NodeServices/HostingModels/HttpNodeInstance.cs index c1fdbaf..1b59e87 100644 --- a/src/Microsoft.AspNetCore.NodeServices/HostingModels/HttpNodeInstance.cs +++ b/src/Microsoft.AspNetCore.NodeServices/HostingModels/HttpNodeInstance.cs @@ -16,9 +16,6 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels /// port number is specified as a constructor parameter), and signals which port was selected using the same /// input/output-based mechanism that the base class uses to determine when the child process is ready to /// accept RPC invocations. - /// - /// TODO: Remove the file-watching logic from here and centralise it in OutOfProcessNodeInstance, implementing - /// the actual watching in .NET code (not Node), for consistency across platforms. /// /// internal class HttpNodeInstance : OutOfProcessNodeInstance @@ -35,26 +32,21 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels private bool _disposed; private int _portNumber; - public HttpNodeInstance(string projectPath, int port = 0, string[] watchFileExtensions = null) + public HttpNodeInstance(string projectPath, string[] watchFileExtensions, int port = 0) : base( EmbeddedResourceReader.Read( typeof(HttpNodeInstance), "/Content/Node/entrypoint-http.js"), projectPath, - MakeCommandLineOptions(port, watchFileExtensions)) + watchFileExtensions, + MakeCommandLineOptions(port)) { _client = new HttpClient(); } - private static string MakeCommandLineOptions(int port, string[] watchFileExtensions) + private static string MakeCommandLineOptions(int port) { - var result = "--port " + port; - if (watchFileExtensions != null && watchFileExtensions.Length > 0) - { - result += " --watch " + string.Join(",", watchFileExtensions); - } - - return result; + return $"--port {port}"; } protected override async Task InvokeExportAsync(NodeInvocationInfo invocationInfo) diff --git a/src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs b/src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs index ac1b691..c45c100 100644 --- a/src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs +++ b/src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs @@ -1,6 +1,7 @@ using System; using System.Diagnostics; using System.IO; +using System.Linq; using System.Threading.Tasks; namespace Microsoft.AspNetCore.NodeServices.HostingModels @@ -18,17 +19,24 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels public abstract class OutOfProcessNodeInstance : INodeInstance { private const string ConnectionEstablishedMessage = "[Microsoft.AspNetCore.NodeServices:Listening]"; - private const string NeedsRestartMessage = "[Microsoft.AspNetCore.NodeServices:Restart]"; private readonly TaskCompletionSource _connectionIsReadySource = new TaskCompletionSource(); private bool _disposed; private readonly StringAsTempFile _entryPointScript; + private FileSystemWatcher _fileSystemWatcher; private readonly Process _nodeProcess; private bool _nodeProcessNeedsRestart; + private readonly string[] _watchFileExtensions; - public OutOfProcessNodeInstance(string entryPointScript, string projectPath, string commandLineArguments = null) + public OutOfProcessNodeInstance( + string entryPointScript, + string projectPath, + string[] watchFileExtensions, + string commandLineArguments) { _entryPointScript = new StringAsTempFile(entryPointScript); _nodeProcess = LaunchNodeProcess(_entryPointScript.FileName, projectPath, commandLineArguments); + _watchFileExtensions = watchFileExtensions; + _fileSystemWatcher = BeginFileWatcher(projectPath); ConnectToInputOutputStreams(); } @@ -80,6 +88,7 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels if (disposing) { _entryPointScript.Dispose(); + EnsureFileSystemWatcherIsDisposed(); } // Make sure the Node process is finished @@ -93,6 +102,15 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels } } + private void EnsureFileSystemWatcherIsDisposed() + { + if (_fileSystemWatcher != null) + { + _fileSystemWatcher.Dispose(); + _fileSystemWatcher = null; + } + } + private static Process LaunchNodeProcess(string entryPointFilename, string projectPath, string commandLineArguments) { var startInfo = new ProcessStartInfo("node") @@ -143,13 +161,6 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels _connectionIsReadySource.SetResult(null); initializationIsCompleted = true; } - else if (evt.Data == NeedsRestartMessage) - { - // Temporarily, the file-watching logic is in Node, so look out for the - // signal that we need to restart. This can be removed once the file-watching - // logic is moved over to the .NET side. - _nodeProcessNeedsRestart = true; - } else if (evt.Data != null) { OnOutputDataReceived(evt.Data); @@ -177,6 +188,69 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels _nodeProcess.BeginErrorReadLine(); } + private FileSystemWatcher BeginFileWatcher(string rootDir) + { + if (_watchFileExtensions == null || _watchFileExtensions.Length == 0) + { + // Nothing to watch + return null; + } + + var watcher = new FileSystemWatcher(rootDir) + { + IncludeSubdirectories = true, + NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.DirectoryName + }; + watcher.Changed += OnFileChanged; + watcher.Created += OnFileChanged; + watcher.Deleted += OnFileChanged; + watcher.Renamed += OnFileRenamed; + watcher.EnableRaisingEvents = true; + return watcher; + } + + private void OnFileChanged(object source, FileSystemEventArgs e) + { + if (IsFilenameBeingWatched(e.FullPath)) + { + RestartDueToFileChange(e.FullPath); + } + } + + private void OnFileRenamed(object source, RenamedEventArgs e) + { + if (IsFilenameBeingWatched(e.OldFullPath) || IsFilenameBeingWatched(e.FullPath)) + { + RestartDueToFileChange(e.OldFullPath); + } + } + + private bool IsFilenameBeingWatched(string fullPath) + { + if (string.IsNullOrEmpty(fullPath)) + { + return false; + } + else + { + var actualExtension = Path.GetExtension(fullPath) ?? string.Empty; + return _watchFileExtensions.Any(actualExtension.Equals); + } + } + + private void RestartDueToFileChange(string fullPath) + { + // TODO: Use proper logger + Console.WriteLine($"Node will restart because file changed: {fullPath}"); + + _nodeProcessNeedsRestart = true; + + // There's no need to watch for any more changes, since we're already restarting, and if the + // restart takes some time (e.g., due to connection draining), we could end up getting duplicate + // notifications. + EnsureFileSystemWatcherIsDisposed(); + } + ~OutOfProcessNodeInstance() { Dispose(false); diff --git a/src/Microsoft.AspNetCore.NodeServices/HostingModels/SocketNodeInstance.cs b/src/Microsoft.AspNetCore.NodeServices/HostingModels/SocketNodeInstance.cs index 7a411a8..3fb25f0 100644 --- a/src/Microsoft.AspNetCore.NodeServices/HostingModels/SocketNodeInstance.cs +++ b/src/Microsoft.AspNetCore.NodeServices/HostingModels/SocketNodeInstance.cs @@ -21,9 +21,6 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels /// The address of the pipe/socket is selected randomly here on the .NET side and sent to the child process as a /// command-line argument (the address space is wide enough that there's no real risk of a clash, unlike when /// selecting TCP port numbers). - /// - /// TODO: Remove the file-watching logic from here and centralise it in OutOfProcessNodeInstance, implementing - /// the actual watching in .NET code (not Node), for consistency across platforms. /// /// internal class SocketNodeInstance : OutOfProcessNodeInstance @@ -38,16 +35,15 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels private StreamConnection _physicalConnection; private string _socketAddress; private VirtualConnectionClient _virtualConnectionClient; - private readonly string[] _watchFileExtensions; public SocketNodeInstance(string projectPath, string[] watchFileExtensions, string socketAddress): base( EmbeddedResourceReader.Read( typeof(SocketNodeInstance), "/Content/Node/entrypoint-socket.js"), projectPath, - MakeNewCommandLineOptions(socketAddress, watchFileExtensions)) + watchFileExtensions, + MakeNewCommandLineOptions(socketAddress)) { - _watchFileExtensions = watchFileExtensions; _socketAddress = socketAddress; } @@ -188,15 +184,9 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels } } - private static string MakeNewCommandLineOptions(string listenAddress, string[] watchFileExtensions) + private static string MakeNewCommandLineOptions(string listenAddress) { - var result = "--listenAddress " + listenAddress; - if (watchFileExtensions != null && watchFileExtensions.Length > 0) - { - result += " --watch " + string.Join(",", watchFileExtensions); - } - - return result; + return $"--listenAddress {listenAddress}"; } #pragma warning disable 649 // These properties are populated via JSON deserialization diff --git a/src/Microsoft.AspNetCore.NodeServices/TypeScript/HttpNodeInstanceEntryPoint.ts b/src/Microsoft.AspNetCore.NodeServices/TypeScript/HttpNodeInstanceEntryPoint.ts index b28f38b..3be759a 100644 --- a/src/Microsoft.AspNetCore.NodeServices/TypeScript/HttpNodeInstanceEntryPoint.ts +++ b/src/Microsoft.AspNetCore.NodeServices/TypeScript/HttpNodeInstanceEntryPoint.ts @@ -3,17 +3,11 @@ import * as http from 'http'; import * as path from 'path'; import { parseArgs } from './Util/ArgsUtil'; -import { autoQuitOnFileChange } from './Util/AutoQuit'; // Webpack doesn't support dynamic requires for files not present at compile time, so grab a direct // reference to Node's runtime 'require' function. const dynamicRequire: (name: string) => any = eval('require'); -const parsedArgs = parseArgs(process.argv); -if (parsedArgs.watch) { - autoQuitOnFileChange(process.cwd(), parsedArgs.watch.split(',')); -} - const server = http.createServer((req, res) => { readRequestBodyAsJson(req, bodyJson => { let hasSentResult = false; @@ -68,6 +62,7 @@ const server = http.createServer((req, res) => { }); }); +const parsedArgs = parseArgs(process.argv); const requestedPortOrZero = parsedArgs.port || 0; // 0 means 'let the OS decide' server.listen(requestedPortOrZero, 'localhost', function () { // Signal to HttpNodeHost which port it should make its HTTP connections on diff --git a/src/Microsoft.AspNetCore.NodeServices/TypeScript/SocketNodeInstanceEntryPoint.ts b/src/Microsoft.AspNetCore.NodeServices/TypeScript/SocketNodeInstanceEntryPoint.ts index 78e0058..0aec541 100644 --- a/src/Microsoft.AspNetCore.NodeServices/TypeScript/SocketNodeInstanceEntryPoint.ts +++ b/src/Microsoft.AspNetCore.NodeServices/TypeScript/SocketNodeInstanceEntryPoint.ts @@ -5,16 +5,11 @@ import * as path from 'path'; import * as readline from 'readline'; import { Duplex } from 'stream'; import { parseArgs } from './Util/ArgsUtil'; -import { autoQuitOnFileChange } from './Util/AutoQuit'; import * as virtualConnectionServer from './VirtualConnections/VirtualConnectionServer'; // Webpack doesn't support dynamic requires for files not present at compile time, so grab a direct // reference to Node's runtime 'require' function. const dynamicRequire: (name: string) => any = eval('require'); -const parsedArgs = parseArgs(process.argv); -if (parsedArgs.watch) { - autoQuitOnFileChange(process.cwd(), parsedArgs.watch.split(',')); -} // Signal to the .NET side when we're ready to accept invocations const server = net.createServer().on('listening', () => { @@ -69,6 +64,7 @@ virtualConnectionServer.createInterface(server).on('connection', (connection: Du // Begin listening now. The underlying transport varies according to the runtime platform. // On Windows it's Named Pipes; on Linux/OSX it's Domain Sockets. const useWindowsNamedPipes = /^win/.test(process.platform); +const parsedArgs = parseArgs(process.argv); const listenAddress = (useWindowsNamedPipes ? '\\\\.\\pipe\\' : '/tmp/') + parsedArgs.listenAddress; server.listen(listenAddress); diff --git a/src/Microsoft.AspNetCore.NodeServices/TypeScript/Util/AutoQuit.ts b/src/Microsoft.AspNetCore.NodeServices/TypeScript/Util/AutoQuit.ts deleted file mode 100644 index 5c75c1c..0000000 --- a/src/Microsoft.AspNetCore.NodeServices/TypeScript/Util/AutoQuit.ts +++ /dev/null @@ -1,18 +0,0 @@ -import * as fs from 'fs'; -import * as path from 'path'; - -export function autoQuitOnFileChange(rootDir: string, extensions: string[]) { - // Note: This will only work on Windows/OS X, because the 'recursive' option isn't supported on Linux. - // Consider using a different watch mechanism (though ideally without forcing further NPM dependencies). - fs.watch(rootDir, { persistent: false, recursive: true } as any, (event, filename) => { - var ext = path.extname(filename); - if (extensions.indexOf(ext) >= 0) { - console.log('Restarting due to file change: ' + filename); - - // Temporarily, the file-watching logic is in Node, so we signal it's time to restart by - // sending the following message back to .NET. Soon the file-watching logic will move over - // to the .NET side, and this whole file can be removed. - console.log('[Microsoft.AspNetCore.NodeServices:Restart]'); - } - }); -} diff --git a/src/Microsoft.AspNetCore.NodeServices/Util/StringAsTempFile.cs b/src/Microsoft.AspNetCore.NodeServices/Util/StringAsTempFile.cs index a9f04a9..4458b77 100644 --- a/src/Microsoft.AspNetCore.NodeServices/Util/StringAsTempFile.cs +++ b/src/Microsoft.AspNetCore.NodeServices/Util/StringAsTempFile.cs @@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.NodeServices { if (disposing) { - // TODO: dispose managed state (managed objects). + // Would dispose managed state here, if there was any } File.Delete(FileName);