Skip to content

#566 No IntelliSense after npm install with a new project #586

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

Merged
mousetraps merged 1 commit into from
Nov 12, 2015
Merged

#566 No IntelliSense after npm install with a new project #586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions Nodejs/Product/Nodejs/Project/NodejsFileNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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));

Expand Down
52 changes: 5 additions & 47 deletions Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ class NodejsProjectNode : CommonProjectNode, VsWebSite.VSWebSite, INodePackageMo

// We delay analysis until things calm down in the node_modules folder.
internal Queue<NodejsFileNode> DelayedAnalysisQueue = new Queue<NodejsFileNode>();
private FileWatcher _nodeModulesWatcher;
private readonly string[] _watcherExtensions = { ".js", ".json" };
private object _idleNodeModulesLock = new object();
Copy link
Copy Markdown
Contributor

@paulvanbrenk paulvanbrenk Nov 12, 2015

Choose a reason for hiding this comment

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

👍

private volatile bool _isIdleNodeModules = false;
private Timer _idleNodeModulesTimer;
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -694,7 +657,7 @@ protected internal override void ProcessReferences() {
if (null == ModulesNode) {
ModulesNode = new NodeModulesNode(this);
AddChild(ModulesNode);
CreateIdleNodeModulesWatcher();
_idleNodeModulesTimer = new Timer(OnIdleNodeModules);
Copy link
Copy Markdown
Contributor

@paulvanbrenk paulvanbrenk Nov 12, 2015

Choose a reason for hiding this comment

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

I wouldn't create a new timer each time, but reset/start the existing timer... they are not cheap.

Copy link
Copy Markdown
Contributor Author

@mousetraps mousetraps Nov 12, 2015

Choose a reason for hiding this comment

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

It's not getting created each time - this only happens when ModulesNode is null, which is when the project hasn't been loaded yet. The timer is reset here: https://github.com/mousetraps/nodejstools/blob/i566/Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs#L113

Copy link
Copy Markdown
Contributor

@paulvanbrenk paulvanbrenk Nov 12, 2015

Choose a reason for hiding this comment

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

Cool, the things you miss when only looking at the diff...

}
}

Expand Down Expand Up @@ -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;
Expand Down