Skip to content

Commit 323f276

Browse files
committed
#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.
1 parent 45f81bb commit 323f276

2 files changed

Lines changed: 8 additions & 49 deletions

File tree

Nodejs/Product/Nodejs/Project/NodejsFileNode.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public NodejsFileNode(NodejsProjectNode root, ProjectElement e)
3232
CreateWatcher(Url);
3333
#endif
3434
if (Url.Contains(AnalysisConstants.NodeModulesFolder)) {
35-
root.DelayedAnalysisQueue.Enqueue(this);
35+
root.EnqueueForDelayedAnalysis(this);
3636
} else {
3737
Analyze();
3838
}
@@ -50,7 +50,8 @@ internal bool ShouldAnalyze {
5050
get {
5151
// We analyze if we are a member item or the file is included
5252
// Also, it should either be marked as compile or not have an item type name (value is null for node_modules
53-
return !ProjectMgr.DelayedAnalysisQueue.Contains(this) &&
53+
return !Url.Contains(NodejsConstants.NodeModulesStagingFolder) &&
54+
!ProjectMgr.DelayedAnalysisQueue.Contains(this) &&
5455
(!IsNonMemberItem || ProjectMgr.IncludeNodejsFile(this)) &&
5556
(ItemNode.ItemTypeName == ProjectFileConstants.Compile || string.IsNullOrEmpty(ItemNode.ItemTypeName));
5657

Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs

Lines changed: 5 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,6 @@ class NodejsProjectNode : CommonProjectNode, VsWebSite.VSWebSite, INodePackageMo
5252

5353
// We delay analysis until things calm down in the node_modules folder.
5454
internal Queue<NodejsFileNode> DelayedAnalysisQueue = new Queue<NodejsFileNode>();
55-
private FileWatcher _nodeModulesWatcher;
56-
private readonly string[] _watcherExtensions = { ".js", ".json" };
5755
private object _idleNodeModulesLock = new object();
5856
private volatile bool _isIdleNodeModules = false;
5957
private Timer _idleNodeModulesTimer;
@@ -81,35 +79,6 @@ public VsProjectAnalyzer Analyzer {
8179
}
8280
}
8381

84-
private void CreateIdleNodeModulesWatcher() {
85-
try {
86-
_idleNodeModulesTimer = new Timer(OnIdleNodeModules);
87-
88-
// This handles the case where there are multiple node_modules folders in a project.
89-
_nodeModulesWatcher = new FileWatcher(ProjectHome) {
90-
IncludeSubdirectories = true,
91-
NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.CreationTime,
92-
EnableRaisingEvents = true
93-
};
94-
95-
_nodeModulesWatcher.Changed += OnNodeModulesWatcherChanged;
96-
_nodeModulesWatcher.Created += OnNodeModulesWatcherChanged;
97-
_nodeModulesWatcher.Deleted += OnNodeModulesWatcherChanged;
98-
99-
RestartFileSystemWatcherTimer();
100-
} catch (Exception ex) {
101-
if (_nodeModulesWatcher != null) {
102-
_nodeModulesWatcher.Dispose();
103-
}
104-
105-
if (ex is IOException || ex is ArgumentException) {
106-
Debug.WriteLine("Error starting FileWatcher:\r\n{0}", ex);
107-
} else {
108-
throw;
109-
}
110-
}
111-
}
112-
11382
private void OnIdleNodeModules(object state) {
11483
lock (_idleNodeModulesLock) {
11584
_isIdleNodeModules = true;
@@ -128,18 +97,12 @@ private void OnIdleNodeModules(object state) {
12897
}
12998
}
13099

131-
private void OnNodeModulesWatcherChanged(object sender, FileSystemEventArgs e) {
132-
try {
133-
var extension = Path.GetExtension(e.FullPath);
134-
if (e.FullPath.Contains(NodejsConstants.NodeModulesFolder) && _watcherExtensions.Any(extension.Equals)) {
135-
RestartFileSystemWatcherTimer();
136-
}
137-
} catch (ArgumentException) {
138-
// Occurs for invalid characters in the filepath. Don't bother restarting the idle timer.
139-
}
100+
internal void EnqueueForDelayedAnalysis(NodejsFileNode fileNode) {
101+
DelayedAnalysisQueue.Enqueue(fileNode);
102+
RestartIdleNodeModulesTimer();
140103
}
141104

142-
private void RestartFileSystemWatcherTimer() {
105+
private void RestartIdleNodeModulesTimer() {
143106
lock (_idleNodeModulesLock) {
144107
_isIdleNodeModules = false;
145108

@@ -694,7 +657,7 @@ protected internal override void ProcessReferences() {
694657
if (null == ModulesNode) {
695658
ModulesNode = new NodeModulesNode(this);
696659
AddChild(ModulesNode);
697-
CreateIdleNodeModulesWatcher();
660+
_idleNodeModulesTimer = new Timer(OnIdleNodeModules);
698661
}
699662
}
700663

@@ -984,11 +947,6 @@ protected override void Dispose(bool disposing) {
984947
_idleNodeModulesTimer.Dispose();
985948
}
986949
_idleNodeModulesTimer = null;
987-
988-
// Unsubscribe event handlers that trigger _idleNodeModulesTimer.
989-
_nodeModulesWatcher.Changed -= OnNodeModulesWatcherChanged;
990-
_nodeModulesWatcher.Created -= OnNodeModulesWatcherChanged;
991-
_nodeModulesWatcher.Deleted -= OnNodeModulesWatcherChanged;
992950
}
993951

994952
NodejsPackage.Instance.IntellisenseOptionsPage.SaveToDiskChanged -= IntellisenseOptionsPageSaveToDiskChanged;

0 commit comments

Comments
 (0)