Merge pull request #502 from scratchfoundation/better-debugging

Better debugging

.github/PULL_REQUEST_TEMPLATE.md

@@ -22,14 +22,14 @@ There are two situations in which we create manual PRs to update translations:
 
 ### Adding a language
 
-* [ ] Edit `src/supported-locales.js`:
+* [ ] Edit `src/supported-locales.mjs`:
   * [ ] Add entry for the language in the `locales` const
   * [ ] Check if language is right-to-left. If so:
     * Add entry in `rtlLocales`
 
 * [ ] Check if the new language uses a country code
   * Check [https://www.transifex.com/explore/languages](https://www.transifex.com/explore/languages). If the language has a country code:
-  * [ ] Edit `src/supported-locales.js`:
+  * [ ] Edit `src/supported-locales.mjs`:
     * Add new entry to `localeMap`. Format is `'<W3C HTML browser locale string>': '<Transifex ICU locale string>'`
   * [ ] Edit `.tx/config`:
     * Add to the `lang_map` list. Format is `<Transifex ICU locale string>:<W3C HTML browser locale string>`
@@ -42,14 +42,14 @@ There are two situations in which we create manual PRs to update translations:
 * [ ] Check if locale is in `react-intl`
   * Look in [https://unpkg.com/react-intl/locale-data/](https://unpkg.com/react-intl/locale-data/)
   * If not in `react-intl`:
-    * [ ] Edit `src/supported-locales.js`:
+    * [ ] Edit `src/supported-locales.mjs`:
       * In `customLocales`, add entry with parent set to a `react-intl` locale
     * [ ] Edit `src/index.js`:
       * In `localeData`, add entry for parent locale
 
 * [ ] Update translations per the "Updating translations" section above
 * [ ] Confirm that we see changes to:
-    * [ ] `src/supported-locales.js`
+    * [ ] `src/supported-locales.mjs`
     * [ ] `src/index.js`
     * [ ] `.tx/config` (if language needed a new locale)
     * [ ] Multiple files like `editor/<resource>/<lang code>.json`

.nvmrc

@@ -1 +1 @@
-v16
+20

lib/progress-logger.mjs

@@ -0,0 +1,42 @@
+/**
+ * Helper class to log progress.
+ */
+export class ProgressLogger {
+    /**
+     * @param {number} [total] Optional: expected total number of items to process.
+     */
+    constructor (total) {
+        this.total = total;
+        this.completed = 0;
+    }
+
+    /**
+     * Set the expected total number of items to process.
+     * @param {number} total Total number of items to process.
+     */
+    setTotal (total) {
+        if (this.total !== total) {
+            this.total = total;
+            delete this.percent;
+        }
+    }
+
+    /**
+     * Increment the number of items processed and log progress.
+     * If a total is set, progress is logged as a percentage and only when the percentage changes.
+     * If no total is set, progress is logged as a count.
+     * @param {number} [count=1] Number of items processed.
+     */
+    increment (count = 1) {
+        this.completed += count;
+        if (this.total) {
+            const percent = Math.floor(100 * this.completed / this.total);
+            if (percent !== this.percent) {
+                this.percent = percent;
+                console.info(`Progress: ${this.percent}% (${this.completed}/${this.total})`);
+            }
+        } else {
+            console.info(`Progress: ${this.completed} of unknown total`);
+        }
+    }
+}

lib/transifex.js

@@ -1,4 +1,4 @@
-#!/usr/bin/env babel-node
+#!/usr/bin/env node
 
 /**
  * @fileoverview
@@ -8,6 +8,10 @@
 const transifexApi = require('@transifex/api').transifexApi;
 const download = require('download');
 
+/**
+ * @import {Collection, JsonApiResource} from '@transifex/api';
+ */
+
 const ORG_NAME = 'llk';
 const SOURCE_LOCALE = 'en';
 
@@ -22,15 +26,44 @@ try {
     throw err;
 }
 
+/*
+ * The Transifex JS API wraps the Transifex JSON API, and is built around the concept of a `Collection`.
+ * A `Collection` begins as a URL builder: methods like `filter` and `sort` add query parameters to the URL.
+ * The `download` method doesn't actually download anything: it returns the built URL. It seems to be intended
+ * primarily for internal use, but shows up in the documentation despite not being advertised in the .d.ts file.
+ * The `download` method is mainly used to skip the `fetch` method in favor of downloading the resource yourself.
+ * The `fetch` method sends a request to the URL and returns a promise that resolves to the first page of results.
+ * If there's only one page of results, the `data` property of the collection object will be an array of all results.
+ * However, if there are multiple pages of results, the `data` property will only contain the first page of results.
+ * Previous versions of this code would unsafely assume that the `data` property contained all results.
+ * The `all` method returns an async iterator that yields all results, fetching additional pages as needed.
+ */
+
+/**
+ * Collects all resources from all pages of a potentially-paginated Transifex collection.
+ * It's not necessary, but also not harmful, to call `fetch()` on the collection before calling this function.
+ * @param {Collection} collection A collection of Transifex resources.
+ * @returns {Promise<JsonApiResource[]>} An array of all resources in the collection.
+ */
+const collectAll = async function (collection) {
+    await collection.fetch(); // fetch the first page if it hasn't already been fetched
+    const collected = [];
+    for await (const item of collection.all()) {
+        collected.push(item);
+    }
+    return collected;
+};
+
 /**
  * Creates a download event for a specific project, resource, and locale.
+ * Returns the URL to download the resource.
  * @param {string} projectSlug - project slug (for example,  "scratch-editor")
  * @param {string} resourceSlug - resource slug (for example,  "blocks")
  * @param {string} localeCode - language code (for example,  "ko")
  * @param {string} mode - translation status of strings to include
- * @returns {Promise<string>} - id of the created download event
+ * @returns {Promise<string>} - URL to download the resource
  */
-const downloadResource = async function (projectSlug, resourceSlug, localeCode, mode = 'default') {
+const getResourceLocation = async function (projectSlug, resourceSlug, localeCode, mode = 'default') {
     const resource = {
         data: {
             id: `o:${ORG_NAME}:p:${projectSlug}:r:${resourceSlug}`,
@@ -66,21 +99,38 @@ const downloadResource = async function (projectSlug, resourceSlug, localeCode,
  * @param {string} resource - resource slug (for example,  "blocks")
  * @param {string} locale - language code (for example,  "ko")
  * @param {string} mode - translation status of strings to include
- * @returns {Promise<object>} - JSON object of translated resource strings (or, of the original resourse
+ * @returns {Promise<object>} - JSON object of translated resource strings (or, of the original resource
  * strings, if the local is the source language)
  */
 const txPull = async function (project, resource, locale, mode = 'default') {
-    const url = await downloadResource(project, resource, locale, mode);
     let buffer;
-    for (let i = 0; i < 5; i++) {
-        try {
-            buffer = await download(url);
-            return JSON.parse(buffer.toString());
-        } catch (e) {
-            process.stdout.write(`got ${e.message}, retrying after ${i + 1} failed attempt(s)\n`);
+    try {
+        const url = await getResourceLocation(project, resource, locale, mode);
+        for (let i = 0; i < 5; i++) {
+            if (i > 0) {
+                console.log(`Retrying txPull download after ${i} failed attempt(s)`);
+            }
+            try {
+                buffer = await download(url); // might throw(?)
+                break;
+            } catch (e) {
+                console.error(e, {project, resource, locale, buffer});
+            }
         }
+        if (!buffer) {
+            throw Error(`txPull download failed after 5 retries: ${url}`);
+        }
+        buffer = buffer.toString();
+        return JSON.parse(buffer);
+    } catch (e) {
+        e.cause = {
+            project,
+            resource,
+            locale,
+            buffer
+        };
+        throw e;
     }
-    throw Error('failed to pull after 5 retries');
 };
 
 /**
@@ -89,13 +139,13 @@ const txPull = async function (project, resource, locale, mode = 'default') {
  * @returns {Promise<array>} - array of strings, slugs identifying each resource in the project
  */
 const txResources = async function (project) {
-    const resources = await transifexApi.Resource.filter({
+    const resources = transifexApi.Resource.filter({
         project: `o:${ORG_NAME}:p:${project}`
     });
 
-    await resources.fetch();
+    const resourcesData = await collectAll(resources);
 
-    const slugs = resources.data.map(r =>
+    const slugs = resourcesData.map(r =>
         // r.id is a longer id string, like "o:llk:p:scratch-website:r:about-l10njson"
         // We just want the slug that comes after ":r:" ("about-l10njson")
         r.id.split(':r:')[1]
@@ -105,15 +155,14 @@ const txResources = async function (project) {
 
 /**
  * @param {string} project - project slug (for example)
- * @returns {object[]} - array of resource objects
+ * @returns {Promise<JsonApiResource[]>} - array of resource objects
  */
 const txResourcesObjects = async function (project) {
-    const resources = await transifexApi.Resource.filter({
+    const resources = transifexApi.Resource.filter({
         project: `o:${ORG_NAME}:p:${project}`
     });
 
-    await resources.fetch();
-    return resources.data;
+    return collectAll(resources);
 };
 
 /**
@@ -128,9 +177,8 @@ const txAvailableLanguages = async function (slug) {
     });
 
     const languages = await project.fetch('languages');
-    await languages.fetch();
-    return languages.data.map(l => l.attributes.code);
-
+    const languagesData = await collectAll(languages);
+    return languagesData.map(l => l.attributes.code);
 };
 
 /**

package.json

@@ -3,38 +3,38 @@
   "version": "3.18.357",
   "description": "Localization for the Scratch 3.0 components",
   "main": "./dist/l10n.js",
-  "browser": "./src/index.js",
+  "browser": "./src/index.mjs",
   "bin": {
     "build-i18n-src": "./scripts/build-i18n-src.js",
     "tx-push-src": "./scripts/tx-push-src.js"
   },
   "scripts": {
     "build": "npm run clean && npm run build:data && webpack --progress --colors --bail",
-    "build:data": "babel-node scripts/build-data",
+    "build:data": "node scripts/build-data.mjs",
     "clean": "rimraf ./dist ./locales && mkdirp dist locales",
     "lint": "npm run lint:js && npm run lint:json",
-    "lint:js": "eslint . --ext .js",
+    "lint:js": "eslint . --ext .js,.mjs,.cjs",
     "lint:json": "jshint -e .json www editor/blocks editor/extensions editor/interface editor/paint-editor",
     "prepare": "husky install",
-    "pull:blocks": "babel-node scripts/tx-pull-editor scratch-editor blocks ./editor/blocks/",
+    "pull:blocks": "node scripts/tx-pull-editor.mjs scratch-editor blocks ./editor/blocks/",
     "pull:editor": "npm run pull:blocks && npm run pull:extensions && npm run pull:paint && npm run pull:interface",
-    "pull:extensions": "babel-node scripts/tx-pull-editor scratch-editor extensions ./editor/extensions/",
+    "pull:extensions": "node scripts/tx-pull-editor.mjs scratch-editor extensions ./editor/extensions/",
     "pull:help": "npm run pull:help:names && npm run pull:help:articles",
     "pull:help:articles": "./scripts/tx-pull-help-articles.js",
     "pull:help:names": "./scripts/tx-pull-help-names.js",
-    "pull:interface": "babel-node scripts/tx-pull-editor scratch-editor interface ./editor/interface/",
-    "pull:paint": "babel-node scripts/tx-pull-editor scratch-editor paint-editor ./editor/paint-editor/",
-    "pull:www": "babel-node scripts/tx-pull-www ./www",
-    "push:help": "./scripts/tx-push-help.js",
+    "pull:interface": "node scripts/tx-pull-editor.mjs scratch-editor interface ./editor/interface/",
+    "pull:paint": "node scripts/tx-pull-editor.mjs scratch-editor paint-editor ./editor/paint-editor/",
+    "pull:www": "node scripts/tx-pull-www.mjs ./www",
+    "push:help": "./scripts/tx-push-help.mjs",
     "sync:help": "npm run push:help && npm run pull:help",
     "test": "npm run lint:js && npm run validate:editor && npm run validate:www && npm run build && npm run lint:json",
     "update": "scripts/update-translations.sh",
-    "validate:blocks": "babel-node scripts/validate-translations ./editor/blocks/",
+    "validate:blocks": "node scripts/validate-translations.mjs ./editor/blocks/",
     "validate:editor": "npm run validate:blocks && npm run validate:extensions && npm run validate:interface && npm run validate:paint",
-    "validate:extensions": "babel-node scripts/validate-translations ./editor/extensions/ && babel-node scripts/validate-extension-inputs",
-    "validate:interface": "babel-node scripts/validate-translations ./editor/interface/",
-    "validate:paint": "babel-node scripts/validate-translations ./editor/paint-editor/",
-    "validate:www": "babel-node scripts/validate-www ./www"
+    "validate:extensions": "node scripts/validate-translations.mjs ./editor/extensions/ && node scripts/validate-extension-inputs.mjs",
+    "validate:interface": "node scripts/validate-translations.mjs ./editor/interface/",
+    "validate:paint": "node scripts/validate-translations.mjs ./editor/paint-editor/",
+    "validate:www": "node scripts/validate-www.mjs ./www"
   },
   "repository": {
     "type": "git",

scripts/build-data.mjs

@@ -40,12 +40,12 @@ Missing locales are ignored, react-intl will use the default messages for them.
  */
 import * as fs from 'fs';
 import * as path from 'path';
-import {sync as mkdirpSync} from 'mkdirp';
+import mkdirp from 'mkdirp';
 import defaultsDeep from 'lodash.defaultsdeep';
-import locales from '../src/supported-locales.js';
+import locales from '../src/supported-locales.mjs';
 
 const MSGS_DIR = './locales/';
-mkdirpSync(MSGS_DIR);
+mkdirp.sync(MSGS_DIR);
 let missingLocales = [];
 
 const combineJson = (component) => {

scripts/tx-pull-editor.mjs

@@ -1,4 +1,4 @@
-#!/usr/bin/env babel-node
+#!/usr/bin/env node
 
 /**
  * @fileoverview
@@ -29,8 +29,8 @@ if (!process.env.TX_TOKEN || args.length < 3) {
 import fs from 'fs';
 import path from 'path';
 import {txPull} from '../lib/transifex.js';
-import {validateTranslations} from '../lib/validate.js';
-import locales, {localeMap} from '../src/supported-locales.js';
+import {validateTranslations} from '../lib/validate.mjs';
+import locales, {localeMap} from '../src/supported-locales.mjs';
 import {batchMap} from '../lib/batch.js';
 
 // Globals

scripts/tx-pull-www.mjs

@@ -1,4 +1,4 @@
-#!/usr/bin/env babel-node
+#!/usr/bin/env node
 
 /**
  * @fileoverview
@@ -27,12 +27,13 @@ if (!process.env.TX_TOKEN || args.length < 1) {
     process.exit(1);
 }
 
-import fs from 'fs';
+import fs from 'fs/promises';
 import path from 'path';
 import mkdirp from 'mkdirp';
 import {txPull, txResources} from '../lib/transifex.js';
-import locales, {localeMap} from '../src/supported-locales.js';
+import locales, {localeMap} from '../src/supported-locales.mjs';
 import {batchMap} from '../lib/batch.js';
+import {ProgressLogger} from '../lib/progress-logger.mjs';
 
 // Globals
 const PROJECT = 'scratch-website';
@@ -46,27 +47,34 @@ const getLocaleData = async function (item) {
     const locale = item.locale;
     const resource = item.resource;
     let txLocale = localeMap[locale] || locale;
-    for (let i = 0; i < 5; i++) {
-        try {
-            const translations = await txPull(PROJECT, resource, txLocale);
-            
-            const txOutdir = `${OUTPUT_DIR}/${PROJECT}.${resource}`;
-            mkdirp.sync(txOutdir);
-            const fileName = `${txOutdir}/${locale}.json`;
-            fs.writeFileSync(
-                fileName,
-                JSON.stringify(translations, null, 4)
-            );
-            return {
-                resource: resource,
-                locale: locale,
-                file: fileName
-            };
-        } catch (e) {
-            process.stdout.write(`got ${e.message}, retrying after ${i + 1} attempt(s)\n`);
-        }
+
+    const translations = await txPull(PROJECT, resource, txLocale);
+
+    const txOutdir = `${OUTPUT_DIR}/${PROJECT}.${resource}`;
+    const fileName = `${txOutdir}/${locale}.json`;
+
+    try {
+        mkdirp.sync(txOutdir);
+        await fs.writeFile(
+            fileName,
+            JSON.stringify(translations, null, 4)
+        );
+
+        return {
+            resource,
+            locale,
+            fileName
+        };
+    } catch (e) {
+        e.cause = {
+            resource,
+            locale,
+            translations,
+            txOutdir,
+            fileName
+        };
+        throw e;
     }
-    throw Error('failed to pull translations after 5 retries');
 };
 
 const expandResourceFiles = (resources) => {
@@ -84,13 +92,21 @@ const expandResourceFiles = (resources) => {
 };
 
 const pullTranslations = async function () {
-    const resources = await txResources('scratch-website');
+    const resources = await txResources(PROJECT);
     const allFiles = expandResourceFiles(resources);
 
+    const progress = new ProgressLogger(allFiles.length);
+
     try {
-        await batchMap(allFiles, CONCURRENCY_LIMIT, getLocaleData);
+        await batchMap(allFiles, CONCURRENCY_LIMIT, async item => {
+            try {
+                await getLocaleData(item);
+            } finally {
+                progress.increment();
+            }
+        });
     } catch (err) {
-        console.error(err); // eslint-disable-line no-console
+        console.error(err);
         process.exit(1);
     }
 };

scripts/tx-push-help.mjs

@@ -1,4 +1,4 @@
-#!/usr/bin/env babel-node
+#!/usr/bin/env node
 
 /**
  * @fileoverview

scripts/validate-extension-inputs.mjs

@@ -1,4 +1,4 @@
-#!/usr/bin/env babel-node
+#!/usr/bin/env node
 
 /**
  * @fileoverview
@@ -9,7 +9,7 @@ import fs from 'fs';
 import path from 'path';
 import async from 'async';
 import assert from 'assert';
-import locales from '../src/supported-locales.js';
+import locales from '../src/supported-locales.mjs';
 
 // Globals
 const JSON_DIR = path.join(process.cwd(), '/editor/extensions');

scripts/validate-translations.mjs

@@ -1,4 +1,4 @@
-#!/usr/bin/env babel-node
+#!/usr/bin/env node
 
 /**
  * @fileoverview
@@ -8,7 +8,7 @@
 const args = process.argv.slice(2);
 const usage = `
  Validate translation json. Usage:
-   babel-node validate_translations.js path
+   node validate_translations.mjs path
      path: where to find the downloaded json files
  `;
 // Fail immediately if the TX_TOKEN is not defined
@@ -19,8 +19,8 @@ if (args.length < 1) {
 import fs from 'fs';
 import path from 'path';
 import async from 'async';
-import {validateTranslations} from '../lib/validate.js';
-import locales from '../src/supported-locales.js';
+import {validateTranslations} from '../lib/validate.mjs';
+import locales from '../src/supported-locales.mjs';
 
 // Globals
 const JSON_DIR = path.resolve(args[0]);

scripts/validate-www.mjs

@@ -1,4 +1,4 @@
-#!/usr/bin/env babel-node
+#!/usr/bin/env node
 
 /**
  * @fileoverview
@@ -8,7 +8,7 @@
 const args = process.argv.slice(2);
 const usage = `
  Validate translation json. Usage:
-   babel-node validate_www.js path
+   node validate_www.mjs path
      path: root folder for all the www resource folders
  `;
 if (args.length < 1) {
@@ -19,8 +19,8 @@ import fs from 'fs';
 import path from 'path';
 import glob from 'glob';
 import async from 'async';
-import {validateTranslations} from '../lib/validate.js';
-import locales from '../src/supported-locales.js';
+import {validateTranslations} from '../lib/validate.mjs';
+import locales from '../src/supported-locales.mjs';
 
 // Globals
 const WWW_DIR = path.resolve(args[0]);

src/index.js

@@ -1,3 +0,0 @@
-import localeData from './locale-data.js';
-import locales, {localeMap, isRtl} from './supported-locales.js';
-export {locales as default, localeData, localeMap, isRtl};

src/index.mjs

@@ -0,0 +1,3 @@
+import localeData from './locale-data.mjs';
+import locales, {localeMap, isRtl} from './supported-locales.mjs';
+export {locales as default, localeData, localeMap, isRtl};

src/locale-data.mjs

@@ -72,7 +72,7 @@ import xh from './locale-data/xh';
 import zh from './locale-data/zh';
 import zu from './locale-data/zu';
 
-import {customLocales} from './supported-locales.js';
+import {customLocales} from './supported-locales.mjs';
 
 let localeData = [].concat(
     en,

webpack.config.js

@@ -21,9 +21,9 @@ module.exports = {
         }]
     },
     entry: {
-        l10n: './src/index.js',
-        supportedLocales: './src/supported-locales.js',
-        localeData: './src/locale-data.js'
+        l10n: './src/index.mjs',
+        supportedLocales: './src/supported-locales.mjs',
+        localeData: './src/locale-data.mjs'
     },
     output: {
         path: path.resolve(__dirname, 'dist'),