From 323f276427df63b01a3c70c4be31f24b0b64d798 Mon Sep 17 00:00:00 2001 From: Sara Itani Date: Wed, 11 Nov 2015 16:00:53 -0800 Subject: [PATCH] #566 No IntelliSense after npm install with a new project This was due to a race condition with the delayed analysis queue. In particular, there were some cases where the NodejsFileNode was created (and therefore an analysis unit enqueued) after the delayed analysis queue had already finished processing existing entries. This fix simplifies the logic to make it more deterministic. In particular, get rid of the separate filewatcher altogether, add entries to the delayed analysis queue whenever new NodejsFileNodes are created, and restart the idle node_modules timer after enqueuing the unit. --- .../Product/Nodejs/Project/NodejsFileNode.cs | 5 +- .../Nodejs/Project/NodejsProjectNode.cs | 52 ++----------------- 2 files changed, 8 insertions(+), 49 deletions(-) diff --git a/Nodejs/Product/Nodejs/Project/NodejsFileNode.cs b/Nodejs/Product/Nodejs/Project/NodejsFileNode.cs index b213f46a6..506402679 100644 --- a/Nodejs/Product/Nodejs/Project/NodejsFileNode.cs +++ b/Nodejs/Product/Nodejs/Project/NodejsFileNode.cs @@ -32,7 +32,7 @@ public NodejsFileNode(NodejsProjectNode root, ProjectElement e) CreateWatcher(Url); #endif if (Url.Contains(AnalysisConstants.NodeModulesFolder)) { - root.DelayedAnalysisQueue.Enqueue(this); + root.EnqueueForDelayedAnalysis(this); } else { Analyze(); } @@ -50,7 +50,8 @@ internal bool ShouldAnalyze { get { // We analyze if we are a member item or the file is included // Also, it should either be marked as compile or not have an item type name (value is null for node_modules - return !ProjectMgr.DelayedAnalysisQueue.Contains(this) && + return !Url.Contains(NodejsConstants.NodeModulesStagingFolder) && + !ProjectMgr.DelayedAnalysisQueue.Contains(this) && (!IsNonMemberItem || ProjectMgr.IncludeNodejsFile(this)) && (ItemNode.ItemTypeName == ProjectFileConstants.Compile || string.IsNullOrEmpty(ItemNode.ItemTypeName)); diff --git a/Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs b/Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs index c36368af9..205860555 100644 --- a/Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs +++ b/Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs @@ -52,8 +52,6 @@ class NodejsProjectNode : CommonProjectNode, VsWebSite.VSWebSite, INodePackageMo // We delay analysis until things calm down in the node_modules folder. internal Queue DelayedAnalysisQueue = new Queue(); - private FileWatcher _nodeModulesWatcher; - private readonly string[] _watcherExtensions = { ".js", ".json" }; private object _idleNodeModulesLock = new object(); private volatile bool _isIdleNodeModules = false; private Timer _idleNodeModulesTimer; @@ -81,35 +79,6 @@ public VsProjectAnalyzer Analyzer { } } - private void CreateIdleNodeModulesWatcher() { - try { - _idleNodeModulesTimer = new Timer(OnIdleNodeModules); - - // This handles the case where there are multiple node_modules folders in a project. - _nodeModulesWatcher = new FileWatcher(ProjectHome) { - IncludeSubdirectories = true, - NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.CreationTime, - EnableRaisingEvents = true - }; - - _nodeModulesWatcher.Changed += OnNodeModulesWatcherChanged; - _nodeModulesWatcher.Created += OnNodeModulesWatcherChanged; - _nodeModulesWatcher.Deleted += OnNodeModulesWatcherChanged; - - RestartFileSystemWatcherTimer(); - } catch (Exception ex) { - if (_nodeModulesWatcher != null) { - _nodeModulesWatcher.Dispose(); - } - - if (ex is IOException || ex is ArgumentException) { - Debug.WriteLine("Error starting FileWatcher:\r\n{0}", ex); - } else { - throw; - } - } - } - private void OnIdleNodeModules(object state) { lock (_idleNodeModulesLock) { _isIdleNodeModules = true; @@ -128,18 +97,12 @@ private void OnIdleNodeModules(object state) { } } - private void OnNodeModulesWatcherChanged(object sender, FileSystemEventArgs e) { - try { - var extension = Path.GetExtension(e.FullPath); - if (e.FullPath.Contains(NodejsConstants.NodeModulesFolder) && _watcherExtensions.Any(extension.Equals)) { - RestartFileSystemWatcherTimer(); - } - } catch (ArgumentException) { - // Occurs for invalid characters in the filepath. Don't bother restarting the idle timer. - } + internal void EnqueueForDelayedAnalysis(NodejsFileNode fileNode) { + DelayedAnalysisQueue.Enqueue(fileNode); + RestartIdleNodeModulesTimer(); } - private void RestartFileSystemWatcherTimer() { + private void RestartIdleNodeModulesTimer() { lock (_idleNodeModulesLock) { _isIdleNodeModules = false; @@ -694,7 +657,7 @@ protected internal override void ProcessReferences() { if (null == ModulesNode) { ModulesNode = new NodeModulesNode(this); AddChild(ModulesNode); - CreateIdleNodeModulesWatcher(); + _idleNodeModulesTimer = new Timer(OnIdleNodeModules); } } @@ -984,11 +947,6 @@ protected override void Dispose(bool disposing) { _idleNodeModulesTimer.Dispose(); } _idleNodeModulesTimer = null; - - // Unsubscribe event handlers that trigger _idleNodeModulesTimer. - _nodeModulesWatcher.Changed -= OnNodeModulesWatcherChanged; - _nodeModulesWatcher.Created -= OnNodeModulesWatcherChanged; - _nodeModulesWatcher.Deleted -= OnNodeModulesWatcherChanged; } NodejsPackage.Instance.IntellisenseOptionsPage.SaveToDiskChanged -= IntellisenseOptionsPageSaveToDiskChanged;