-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
Significantly improved addFile/changeFile/removeFile performance. #3726
base: master
Are you sure you want to change the base?
Changes from all commits
5b38dc0
ec43b0e
a4a5002
77cdf9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,6 +114,7 @@ class FileList { | |
return lastCompletedRefresh | ||
} | ||
|
||
// TODO - Change this so that it's not a getter. Getters shouldn't do lots of processing! | ||
get files () { | ||
const served = [] | ||
const included = {} | ||
|
@@ -172,18 +173,18 @@ class FileList { | |
const excluded = this._findExcluded(path) | ||
if (excluded) { | ||
log.debug(`Add file "${path}" ignored. Excluded by "${excluded}".`) | ||
return this.files | ||
return | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jginsburgn Yes, I think it should be better to land it in a major release. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure |
||
} | ||
|
||
const pattern = this._findIncluded(path) | ||
if (!pattern) { | ||
log.debug(`Add file "${path}" ignored. Does not match any pattern.`) | ||
return this.files | ||
return | ||
} | ||
|
||
if (this._exists(path)) { | ||
log.debug(`Add file "${path}" ignored. Already in the list.`) | ||
return this.files | ||
return | ||
} | ||
|
||
const file = new File(path) | ||
|
@@ -195,7 +196,6 @@ class FileList { | |
|
||
log.info(`Added file "${path}".`) | ||
this._emitModified() | ||
return this.files | ||
} | ||
|
||
async changeFile (path, force) { | ||
|
@@ -204,7 +204,7 @@ class FileList { | |
|
||
if (!file) { | ||
log.debug(`Changed file "${path}" ignored. Does not match any file in the list.`) | ||
return this.files | ||
return | ||
} | ||
|
||
const [stat] = await Promise.all([statAsync(path), this._refreshing]) | ||
|
@@ -214,7 +214,6 @@ class FileList { | |
log.info(`Changed file "${path}".`) | ||
this._emitModified(force) | ||
} | ||
return this.files | ||
} | ||
|
||
async removeFile (path) { | ||
|
@@ -224,12 +223,10 @@ class FileList { | |
if (file) { | ||
helper.arrayRemove(this._getFilesByPattern(pattern.pattern), file) | ||
log.info(`Removed file "${path}".`) | ||
|
||
this._emitModified() | ||
} else { | ||
log.debug(`Removed file "${path}" ignored. Does not match any file in the list.`) | ||
} | ||
return this.files | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
TODO:
instead ofTODO -
:)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi - I really don't want to spend the 15 minutes on context switching to remove a '-'. Can this be accepted as is (assuming no other issues)?