-
-
Notifications
You must be signed in to change notification settings - Fork 496
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
refactor(loops): Clean up some loops #3362
base: main
Are you sure you want to change the base?
Conversation
@zachleat Beep! :) |
@@ -212,7 +212,7 @@ class Template extends TemplateContent { | |||
let results = await Promise.all(promises); | |||
|
|||
permalinkValue = {}; | |||
for (let j = 0, k = keys.length; j < k; j++) { | |||
for (let j = 0; j < keys.length; j++) { |
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.
question: Extracting the length into a variable used to be a performance improvement thing. Is that irrelevant now with newer browser versions?
continue; | ||
} | ||
|
||
for (let item of this.collectionsData[collectionName]) { | ||
for (let item of collectionData) { |
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.
question: Is Line 674 still working as expected? Is there a test for it?
|
||
let matches = paths.filter((path) => { | ||
return !this.extensionMap.hasEngine(path); | ||
}); | ||
return matches; |
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: We might as well return it inline :)
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.
Just revert those two blocks and I’m happy!
for (let path of this.extensionMap.getPassthroughCopyGlobs(this.inputDir)) { | ||
paths.add(path); | ||
} | ||
let paths = new Set( |
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.
Fine with almost all of the style changes here except the .concat ones. I prefer the old method, sorry :-/
uniqueIgnores.add(ignore); | ||
} | ||
let uniqueIgnores = new Set( | ||
this.fileIgnores |
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.
And this one
Lots of small tweaks. Usually to make loops easier to read, and sometimes to replace a loop with something that is (hopefully) easier or faster to read.