Skip to content
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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions src/Benchmark/BenchmarkGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ class BenchmarkGroup {
}

finish(label, totalTimeSpent) {
for (var type in this.benchmarks) {
let bench = this.benchmarks[type];
for (let [type, bench] of Object.entries(this.benchmarks)) {
let isAbsoluteMinimumComparison = this.minimumThresholdMs > 0;
let totalForBenchmark = bench.getTotal();
let percent = Math.round((totalForBenchmark * 100) / totalTimeSpent);
Expand Down
8 changes: 4 additions & 4 deletions src/Benchmark/BenchmarkManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ class BenchmarkManager {
reset() {
this.start = this.getNewTimestamp();

for (var j in this.benchmarkGroups) {
this.benchmarkGroups[j].reset();
for (let group of Object.keys(this.benchmarkGroups)) {
this.benchmarkGroups[group].reset();
}
}

Expand Down Expand Up @@ -64,8 +64,8 @@ class BenchmarkManager {

finish() {
let totalTimeSpentBenchmarking = this.getNewTimestamp() - this.start;
for (var j in this.benchmarkGroups) {
this.benchmarkGroups[j].finish(j, totalTimeSpentBenchmarking);
for (let group of Object.keys(this.benchmarkGroups)) {
this.benchmarkGroups[group].finish(group, totalTimeSpentBenchmarking);
}
}
}
Expand Down
10 changes: 4 additions & 6 deletions src/EleventyExtensionMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,10 @@ class EleventyExtensionMap {
// on paths found from the file system glob search.
// TODO: Method name might just need to be renamed to something more accurate.
isFullTemplateFilePath(path) {
for (let extension of this.validTemplateLanguageKeys) {
if (path.endsWith(`.${extension}`)) {
return true;
}
}
return false;
return this.validTemplateLanguageKeys.some(
(extension) => path.endsWith(`.${extension}`)
)

}

getCustomExtensionEntry(extension) {
Expand Down
45 changes: 16 additions & 29 deletions src/EleventyFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,14 @@ class EleventyFiles {
}

get passthroughGlobs() {
let paths = new Set();
// stuff added in addPassthroughCopy()
for (let path of this.passthroughManager.getConfigPathGlobs()) {
paths.add(path);
}
// non-template language extensions
for (let path of this.extensionMap.getPassthroughCopyGlobs(this.inputDir)) {
paths.add(path);
}
let paths = new Set(
Copy link
Member

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 :-/

// stuff added in addPassthroughCopy()
this.passthroughManager.getConfigPathGlobs()
.concat(
// non-template language extensions
this.extensionMap.getPassthroughCopyGlobs(this.inputDir)
)
);
return Array.from(paths);
}

Expand Down Expand Up @@ -184,13 +183,10 @@ class EleventyFiles {
}

getIgnoreGlobs() {
let uniqueIgnores = new Set();
for (let ignore of this.fileIgnores) {
uniqueIgnores.add(ignore);
}
for (let ignore of this.extraIgnores) {
uniqueIgnores.add(ignore);
}
let uniqueIgnores = new Set(
this.fileIgnores
Copy link
Member

Choose a reason for hiding this comment

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

And this one

.concat(this.extraIgnores),
);
// Placing the config ignores last here is important to the tests
for (let ignore of this.config.ignores) {
uniqueIgnores.add(TemplateGlob.normalizePath(this.localPathRoot || ".", ignore));
Expand Down Expand Up @@ -267,11 +263,9 @@ class EleventyFiles {
}

getIgnores() {
let files = new Set();

for (let ignore of EleventyFiles.getFileIgnores(this.getIgnoreFiles())) {
files.add(ignore);
}
let files = new Set(
EleventyFiles.getFileIgnores(this.getIgnoreFiles())
);

// testing API
if (this.eleventyIgnoreContent !== false) {
Expand Down Expand Up @@ -431,14 +425,7 @@ class EleventyFiles {
if (!filePath) {
return false;
}

for (let path of paths) {
if (path === filePath) {
return true;
}
}

return false;
return paths.some((path) => path === filePath)
}

/* For `eleventy --watch` */
Expand Down
9 changes: 3 additions & 6 deletions src/EleventyWatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,9 @@ class EleventyWatch {
}

hasQueuedFiles(files) {
for (const file of files) {
if (this.hasQueuedFile(file)) {
return true;
}
}
return false;
return files.some(
(file) => this.hasQueuedFile(file)
)
}

get pendingQueue() {
Expand Down
2 changes: 1 addition & 1 deletion src/Template.js
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {

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?

let key = keys[j];
permalinkValue[key] = results[j];
debug(
Expand Down
2 changes: 1 addition & 1 deletion src/TemplateLayout.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ class TemplateLayout extends TemplateContent {

// The parent already includes the rest of the layout chain
let upstreamFns = await layoutTemplate.getCompiledLayoutFunctions();
for (let j = 0, k = upstreamFns.length; j < k; j++) {
for (let j = 0; j < upstreamFns.length; j++) {
fns.push(upstreamFns[j]);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/TemplateMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -653,13 +653,13 @@ class TemplateMap {
}

populateCollectionsWithContent() {
for (let collectionName in this.collectionsData) {
for (let collectionData of Object.values(this.collectionsData)) {
// skip custom collections set in configuration files that have arbitrary types
if (!Array.isArray(this.collectionsData[collectionName])) {
if (!Array.isArray(collectionData)) {
continue;
}

for (let item of this.collectionsData[collectionName]) {
for (let item of collectionData) {

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?

// skip custom collections set in configuration files that have arbitrary types
if (!isPlainObject(item) || !("inputPath" in item)) {
continue;
Expand Down
10 changes: 3 additions & 7 deletions src/TemplatePassthroughManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,9 @@ class TemplatePassthroughManager {
}

getNonTemplatePaths(paths) {
let matches = [];
for (let path of paths) {
if (!this.extensionMap.hasEngine(path)) {
matches.push(path);
}
}

let matches = paths.filter((path) => {
return !this.extensionMap.hasEngine(path);
});
return matches;

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 :)

}

Expand Down
5 changes: 1 addition & 4 deletions src/Util/Objects/ProxyWrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ function wrapObject(target, fallback) {
return Reflect.has(fallback, prop);
},
ownKeys(target) {
let s = new Set();
for (let k of Reflect.ownKeys(target)) {
s.add(k);
}
let s = new Set(Reflect.ownKeys(target));
if (isPlainObject(fallback)) {
for (let k of Reflect.ownKeys(fallback)) {
s.add(k);
Expand Down