From d2abaad756b9bf5b56b7b6aac3d61537701936d1 Mon Sep 17 00:00:00 2001 From: "Queen Vinyl Da.i'gyu-Kazotetsu" Date: Mon, 6 Mar 2023 07:06:45 -0800 Subject: [PATCH] refactor(build): make Document.findAll() asynchronous (#8302) This may help to improve performance. --- build/cli.ts | 4 ++-- content/document.test.ts | 24 +++++++++++----------- content/document.ts | 5 +++-- content/translations.ts | 8 ++++---- markdown/m2h/cli.ts | 2 +- server/translations.ts | 33 +++++++++++++++++-------------- testing/tests/destructive.test.ts | 8 ++++---- tool/cli.ts | 6 +++--- 8 files changed, 48 insertions(+), 42 deletions(-) diff --git a/build/cli.ts b/build/cli.ts index 141c81ff1528..ca58317ce16c 100644 --- a/build/cli.ts +++ b/build/cli.ts @@ -65,7 +65,7 @@ async function buildDocumentInteractive( } if (!interactive) { - const translations = translationsOf(document.metadata); + const translations = await translationsOf(document.metadata); if (translations && translations.length > 0) { document.translations = translations; } else { @@ -133,7 +133,7 @@ async function buildDocuments( const metadata: GlobalMetadata = {}; - const documents = Document.findAll(findAllOptions); + const documents = await Document.findAll(findAllOptions); const progressBar = new cliProgress.SingleBar( {}, cliProgress.Presets.shades_grey diff --git a/content/document.test.ts b/content/document.test.ts index 67a99924a33b..d09fa530c3ab 100644 --- a/content/document.test.ts +++ b/content/document.test.ts @@ -4,13 +4,13 @@ import path from "node:path"; import * as Document from "./document.js"; describe("Document.findAll()", () => { - it("should always return files that exist", () => { - const filePaths = [...Document.findAll().iterPaths()]; + it("should always return files that exist", async () => { + const filePaths = [...(await Document.findAll()).iterPaths()]; expect(filePaths.every((value) => fs.existsSync(value))).toBeTruthy(); }); - it("all files should be either index.html or index.md", () => { - const filePaths = [...Document.findAll().iterPaths()]; + it("all files should be either index.html or index.md", async () => { + const filePaths = [...(await Document.findAll()).iterPaths()]; expect( filePaths.every( (value) => value.endsWith("index.html") || value.endsWith("index.md") @@ -18,19 +18,21 @@ describe("Document.findAll()", () => { ).toBeTruthy(); }); - it("searching by specific file", () => { - const filePaths = [...Document.findAll().iterPaths()]; + it("searching by specific file", async () => { + const filePaths = [...(await Document.findAll()).iterPaths()]; const randomFile = filePaths[Math.floor(Math.random() * filePaths.length)]; const specificFilePaths = [ - ...Document.findAll({ files: new Set([randomFile]) }).iterPaths(), + ...(await Document.findAll({ files: new Set([randomFile]) })).iterPaths(), ]; expect(specificFilePaths).toHaveLength(1); expect(specificFilePaths[0]).toBe(randomFile); }); - it("searching by specific locales", () => { + it("searching by specific locales", async () => { const specificFilePaths = [ - ...Document.findAll({ locales: new Map([["fr", true]]) }).iterPaths(), + ...( + await Document.findAll({ locales: new Map([["fr", true]]) }) + ).iterPaths(), ]; expect( specificFilePaths.every((filePath) => @@ -39,9 +41,9 @@ describe("Document.findAll()", () => { ).toBeTruthy(); }); - it("searching by specific folders", () => { + it("searching by specific folders", async () => { const specificFilePaths = [ - ...Document.findAll({ folderSearch: "/foo/" }).iterPaths(), + ...(await Document.findAll({ folderSearch: "/foo/" })).iterPaths(), ]; expect( specificFilePaths.every((filePath) => diff --git a/content/document.ts b/content/document.ts index ed44212ffe1f..3bcf5a5fe2ec 100644 --- a/content/document.ts +++ b/content/document.ts @@ -443,7 +443,7 @@ export function findByURL( return doc; } -export function findAll({ +export async function findAll({ files = new Set(), folderSearch = null, locales = new Map(), @@ -514,7 +514,8 @@ export function findAll({ return true; }) .crawl(root); - filePaths.push(...(api.sync() as PathsOutput)); + const output: PathsOutput = await api.withPromise(); + filePaths.push(...output); } return { count: filePaths.length, diff --git a/content/translations.ts b/content/translations.ts index c25f4be7f44f..b9f46569da32 100644 --- a/content/translations.ts +++ b/content/translations.ts @@ -10,8 +10,8 @@ const LANGUAGES = new Map( const TRANSLATIONS_OF = new Map(); -function gatherTranslations() { - const iter = Document.findAll().iterDocs(); +async function gatherTranslations() { + const iter = (await Document.findAll()).iterDocs(); for (const { metadata: { slug, locale, title }, } of iter) { @@ -37,11 +37,11 @@ function gatherTranslations() { } } -export function translationsOf({ slug, locale: currentLocale }) { +export async function translationsOf({ slug, locale: currentLocale }) { if (TRANSLATIONS_OF.size === 0) { const label = "Time to gather all translations"; console.time(label); - gatherTranslations(); + await gatherTranslations(); console.timeEnd(label); } const translations = TRANSLATIONS_OF.get(slug.toLowerCase()); diff --git a/markdown/m2h/cli.ts b/markdown/m2h/cli.ts index 433c0dd1cae3..736054920601 100644 --- a/markdown/m2h/cli.ts +++ b/markdown/m2h/cli.ts @@ -49,7 +49,7 @@ program .argument("[folder]", "convert by folder") .action( tryOrExit(async ({ args, options }) => { - const all = Document.findAll({ + const all = await Document.findAll({ folderSearch: args.folder, locales: buildLocaleMap(options.locale), }); diff --git a/server/translations.ts b/server/translations.ts index c6c8190985f7..ae400dafd7c3 100644 --- a/server/translations.ts +++ b/server/translations.ts @@ -60,7 +60,7 @@ function packageTranslationDifferences(translationDifferences) { } const _foundDocumentsCache = new Map(); -export function findDocuments({ locale }) { +export async function findDocuments({ locale }) { const counts = { // Number of documents found that aren't skipped found: 0, @@ -76,7 +76,7 @@ export function findDocuments({ locale }) { const documents = []; const t1 = new Date(); - const documentsFound = Document.findAll({ + const documentsFound = await Document.findAll({ locales: new Map([[locale, true]]), }); counts.total = documentsFound.count; @@ -196,7 +196,7 @@ function getDocument(filePath) { const _defaultLocaleDocumentsCache = new Map(); -function gatherL10NstatsSection({ +async function gatherL10NstatsSection({ locale, mdnSection = "/", subSections = [], @@ -272,7 +272,7 @@ function gatherL10NstatsSection({ locale + mdnSection.toLowerCase() + (mdnSection.endsWith("/") ? "" : "/") ); - const foundTranslations = Document.findAll({ + const foundTranslations = await Document.findAll({ locales: new Map([[locale, true]]), folderSearch, }); @@ -295,7 +295,7 @@ function gatherL10NstatsSection({ (mdnSection.endsWith("/") ? "" : "/") ); - const foundDefaultLocale = Document.findAll({ + const foundDefaultLocale = await Document.findAll({ locales: new Map([[DEFAULT_LOCALE.toLowerCase(), true]]), folderSearch: folderSearchDefaultLocale, }); @@ -404,7 +404,7 @@ function gatherL10NstatsSection({ const _detailsSectionCache = new Map(); -function buildL10nDashboard({ +async function buildL10nDashboard({ locale, section, }: { @@ -420,10 +420,13 @@ function buildL10nDashboard({ } const sectionDirPath = replaceSepPerOS(section); const defaultLocaleDocs = [ - ...Document.findAll({ - locales: new Map([[DEFAULT_LOCALE.toLowerCase(), true]]), - folderSearch: DEFAULT_LOCALE.toLowerCase() + sectionDirPath.toLowerCase(), - }).iterDocs(), + ...( + await Document.findAll({ + locales: new Map([[DEFAULT_LOCALE.toLowerCase(), true]]), + folderSearch: + DEFAULT_LOCALE.toLowerCase() + sectionDirPath.toLowerCase(), + }) + ).iterDocs(), ]; const subSectionsStartingWith = defaultLocaleDocs @@ -449,7 +452,7 @@ function buildL10nDashboard({ }) .map((s) => "/" + s); - const l10nStatsSection = gatherL10NstatsSection({ + const l10nStatsSection = await gatherL10NstatsSection({ locale, mdnSection: section, subSections, @@ -539,7 +542,7 @@ router.get("/", async (req, res) => { return res.status(500).send("CONTENT_TRANSLATED_ROOT not set"); } - const countsByLocale = countFilesByLocale(); + const countsByLocale = await countFilesByLocale(); const locales = [...VALID_LOCALES] .map(([localeLC, locale]) => { @@ -557,7 +560,7 @@ router.get("/", async (req, res) => { res.json({ locales }); }); -function countFilesByLocale() { +async function countFilesByLocale() { const counts = new Map(); let strip = CONTENT_TRANSLATED_ROOT; if (!strip.endsWith(path.sep)) { @@ -596,7 +599,7 @@ router.get("/differences", async (req, res) => { const label = `Find all translated documents (${locale})`; console.time(label); - const found = findDocuments({ locale }); + const found = await findDocuments({ locale }); console.timeEnd(label); res.json(found); }); @@ -641,7 +644,7 @@ router.get("/dashboard", async (req, res) => { const label = `Gather stat for ${locale} and section ${section}`; console.time(label); - const data = buildL10nDashboard({ locale, section }); + const data = await buildL10nDashboard({ locale, section }); console.timeEnd(label); res.json(data); }); diff --git a/testing/tests/destructive.test.ts b/testing/tests/destructive.test.ts index 754904e02808..ef8105a053c0 100644 --- a/testing/tests/destructive.test.ts +++ b/testing/tests/destructive.test.ts @@ -99,10 +99,10 @@ describe("fixing flaws", () => { .split("\n") .filter((line) => regexPattern.test(line)); expect(dryRunNotices).toHaveLength(4); - expect(dryRunNotices[0]).toContain(path.join(pattern, "bad_pre_tags")); - expect(dryRunNotices[1]).toContain(path.join(pattern, "deprecated_macros")); - expect(dryRunNotices[2]).toContain(path.join(pattern, "images")); - expect(dryRunNotices[3]).toContain(pattern); + expect(dryRunNotices[0]).toContain(pattern); + expect(dryRunNotices[1]).toContain(path.join(pattern, "bad_pre_tags")); + expect(dryRunNotices[2]).toContain(path.join(pattern, "deprecated_macros")); + expect(dryRunNotices[3]).toContain(path.join(pattern, "images")); const dryrunFiles = getChangedFiles(tempContentDir); expect(dryrunFiles).toHaveLength(0); }); diff --git a/tool/cli.ts b/tool/cli.ts index 5bdd389b5a10..b4edb0d9dade 100644 --- a/tool/cli.ts +++ b/tool/cli.ts @@ -706,7 +706,7 @@ program tryOrExit(async ({ args, options }: FixFlawsActionParameters) => { const { fixFlawsTypes } = args; const { locale, fileTypes } = options; - const allDocs = Document.findAll({ + const allDocs = await Document.findAll({ locales: new Map([[locale.toLowerCase(), true]]), }); for (const document of allDocs.iterDocs()) { @@ -771,7 +771,7 @@ program if (!fs.existsSync(CONTENT_TRANSLATED_ROOT)) { throw new Error(`${CONTENT_TRANSLATED_ROOT} does not exist`); } - const documents = Document.findAll(); + const documents = await Document.findAll(); if (!documents.count) { throw new Error("No documents to analyze"); } @@ -1001,7 +1001,7 @@ if (Mozilla && !Mozilla.dntEnabled()) { .map((m) => `"${m}"`) .join(", ")} within content folder(s) matching "${foldersearch}"` ); - const documents = Document.findAll({ + const documents = await Document.findAll({ folderSearch: foldersearch, }); if (!documents.count) {