Skip to content
Draft
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
6 changes: 3 additions & 3 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ jobs:
sed -e 's/^\[warn] \(.*\)$/::warning file=\1,line=1::File was not formatted with prettier. Usually, commenting !format will fix this./' \
))

type-warnings:
typecheck:
runs-on: ubuntu-latest
steps:
- name: Checkout
Expand All @@ -77,8 +77,8 @@ jobs:
cache: npm
- name: Install dependencies
run: npm ci
- name: Generate type warnings
run: node development/ci-generate-type-warnings.js
- name: Check types
run: node development/ci-type-check.js
env:
PR_NUMBER: "${{ github.event.number }}"
GH_REPO: "${{ github.repository }}"
84 changes: 70 additions & 14 deletions development/builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import parseMetadata from "./parse-extension-metadata.js";
import parseTranslations from "./parse-extension-translations.js";
import renderTemplate from "./render-template.js";
import renderDocs from "./render-docs.js";
import transpileTypeScript from "./transpile-typescript.js";
import { mkdirp, recursiveReadDirectory } from "./fs-utils.js";
import {
fetchAllDependencies,
Expand Down Expand Up @@ -176,8 +177,27 @@ class ExtensionFile extends BuildFile {
this.mode = mode;
}

getType() {
return ".js";
}

/**
* @returns {{jsCode: string; sourceMap: string | null}}
*/
async transpile() {
const sourceMap = this.mode === "development";
const source = await fsPromises.readFile(this.sourcePath, "utf-8");
if (this.sourcePath.endsWith(".ts")) {
return transpileTypeScript(source, this.slug, !!sourceMap);
}
return {
jsCode: source,
sourceMap: null,
};
}

async read() {
let data = await fsPromises.readFile(this.sourcePath, "utf-8");
let data = (await this.transpile()).jsCode;

if (this.mode !== "development") {
const dependenciesJS = rewriteExternalToInline(data);
Expand All @@ -186,6 +206,11 @@ class ExtensionFile extends BuildFile {
let prefixJS = "";
let suffixJS = "";

if (this.sourcePath.endsWith(".ts")) {
prefixJS +=
"/* transpiled from TypeScript - see repository for original version with types */";
}

const translations = filterTranslationsByPrefix(
this.allTranslations,
`${this.slug}@`
Expand Down Expand Up @@ -286,10 +311,10 @@ class ExtensionFile extends BuildFile {
}
}

validateImports(js);
validateImports((await this.transpile()).jsCode);
}

getStrings() {
async getStrings() {
if (!this.featured) {
return null;
}
Expand All @@ -315,7 +340,7 @@ class ExtensionFile extends BuildFile {
},
};

const jsCode = fs.readFileSync(this.sourcePath, "utf-8");
const jsCode = (await this.transpile()).jsCode;
const unprefixedRuntimeStrings = parseTranslations(jsCode);
const runtimeStrings = Object.fromEntries(
Object.entries(unprefixedRuntimeStrings).map(([key, value]) => [
Expand All @@ -331,9 +356,26 @@ class ExtensionFile extends BuildFile {
}

async getDependencies() {
return parseExtensionDependencies(
await fsPromises.readFile(this.sourcePath, "utf-8")
);
return parseExtensionDependencies((await this.transpile()).jsCode);
}
}

class ExtensionSourceMapFile extends BuildFile {
/**
* @param {ExtensionFile} extensionFile
*/
constructor(extensionFile) {
super(extensionFile.sourcePath);
/** @type {ExtensionFile} */
this.extensionFile = extensionFile;
}

getType() {
return ".json";
}

async read() {
return (await this.extensionFile.transpile()).sourceMap;
}
}

Expand Down Expand Up @@ -675,13 +717,13 @@ class Build {
/**
* @returns {Record<string, Record<string, TranslatableString>>}
*/
generateL10N() {
async generateL10N() {
const allStrings = {};

for (const [filePath, file] of Object.entries(this.files)) {
let fileStrings;
try {
fileStrings = file.getStrings();
fileStrings = await file.getStrings();
} catch (error) {
console.error(error);
throw new Error(
Expand Down Expand Up @@ -717,7 +759,7 @@ class Build {
async exportL10N(root) {
await mkdirp(root);

const groups = this.generateL10N();
const groups = await this.generateL10N();
for (const [name, strings] of Object.entries(groups)) {
const filename = pathUtil.join(root, `exported-${name}.json`);
await fsPromises.writeFile(filename, JSON.stringify(strings, null, 2));
Expand Down Expand Up @@ -799,20 +841,34 @@ class Builder {
for (const [filename, absolutePath] of await recursiveReadDirectory(
this.extensionsRoot
)) {
if (!filename.endsWith(".js")) {
const isJavaScript = filename.endsWith(".js");
const isTypeScript =
filename.endsWith(".ts") && !filename.endsWith(".d.ts");
if (!isJavaScript && !isTypeScript) {
// Not an extension.
continue;
}

const extensionSlug = filename.split(".")[0];
const featured = featuredExtensionSlugs.includes(extensionSlug);
const file = new ExtensionFile(

const extensionFile = new ExtensionFile(
absolutePath,
extensionSlug,
featured,
translations["extension-runtime"],
this.mode
);
extensionFiles[extensionSlug] = file;
build.files[`/${filename}`] = file;
extensionFiles[extensionSlug] = extensionFile;
build.files[`/${extensionSlug}.js`] = extensionFile;

// Sourcemaps are only accurate in development builds as production builds will insert strings
// and cached dependencies.
if (isTypeScript && this.mode === "development") {
build.files[`/${extensionSlug}.js.map`] = new ExtensionSourceMapFile(
extensionFile
);
}
}

/** @type {Record<string, ImageFile>} */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ import ts from "typescript";
import { createAnnotation, getChangedFiles, isCI } from "./ci-interop.js";

/**
* @fileoverview Generates CI annotations for type warnings in files modified by the
* pull request. Standard TypeScript CLI will include all files and its output is
* detected as errors, so not usable for us.
* @fileoverview Generates CI annotations for type checking.
* JavaScript files are warn-only; TypeScript are hard errors.
*/

const check = async () => {
Expand All @@ -16,7 +15,7 @@ const check = async () => {
pathUtil.join(rootDir, f)
);

console.log(`${changedFiles.size} changed files:`);
console.log(`${changedFiles.length} changed files:`);
console.log(Array.from(changedFiles).sort().join("\n"));
console.log("");

Expand All @@ -39,12 +38,25 @@ const check = async () => {
];

let numWarnings = 0;
let numErrors = 0;

for (const diagnostic of diagnostics) {
if (!changedFilesAbsolute.includes(diagnostic.file.fileName)) {
const isBlocker = diagnostic.file.fileName.endsWith(".ts");

if (
!isBlocker &&
!changedFilesAbsolute.includes(diagnostic.file.fileName)
) {
// Warning in a file not touched by the PR: ignore
continue;
}

if (isBlocker) {
numErrors++;
} else {
numWarnings++;
}

const startPosition = ts.getLineAndCharacterOfPosition(
diagnostic.file,
diagnostic.start
Expand All @@ -60,12 +72,12 @@ const check = async () => {
0
);

numWarnings++;
createAnnotation({
type: "warning",
type: isBlocker ? "error" : "warning",
file: diagnostic.file.fileName,
title: "Type warning - may indicate a bug - ignore if no bug",
onlyIfChanged: true,
title: isBlocker
? "Type error - must be fixed"
: "Type warning - may indicate a bug - ignore if no bug",
message: flattened,
line: startPosition.line + 1,
col: startPosition.character + 1,
Expand All @@ -74,12 +86,17 @@ const check = async () => {
});
}

console.log(`Errors: ${numErrors}`);
console.log(`Warnings in changed files: ${numWarnings}`);
console.log(`Warnings in all files: ${diagnostics.length}`);
console.log(`Total errors+warnings in all files: ${diagnostics.length}`);

return numErrors === 0;
};

if (isCI()) {
check();
check().then((success) => {
process.exit(success ? 0 : 1);
});
} else {
console.error(
"This script is only intended to be used in CI. For development, use normal TypeScript CLI instead."
Expand Down
60 changes: 60 additions & 0 deletions development/transpile-typescript.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import * as pathUtil from "node:path";
import ts from "typescript";

/**
* @fileoverview Transpiles TypeScript to JavaScript.
*/

const tsconfigPath = pathUtil.join(import.meta.dirname, "../tsconfig.json");

/**
* @type {import("typescript").CompilerOptions | null}
*/
let cachedBaseOptions = null;

/**
* @returns {import("typescript").CompilerOptions}
*/
const getBaseCompilerOptions = () => {
if (!cachedBaseOptions) {
const configFile = ts.readConfigFile(tsconfigPath, ts.sys.readFile);
if (configFile.error) {
throw new Error(
ts.flattenDiagnosticMessageText(configFile.error.messageText, "\n")
);
}
const parsed = ts.parseJsonConfigFileContent(
configFile.config,
ts.sys,
pathUtil.dirname(tsconfigPath)
);
cachedBaseOptions = parsed.options;
}
return cachedBaseOptions;
};

/**
* Transpile a TypeScript extension to JavaScript. Does not enforce any sort of type checking.
* @param {string} tsCode TypeScript code
* @param {string} slug extension slug
* @param {boolean} sourceMap true to generate source map
* @returns {{jsCode: string, sourcemap: string|null}}
*/
const compileTypeScript = (tsCode, slug, sourceMap) => {
const result = ts.transpileModule(tsCode, {
// used in the generated source map
fileName: `${slug}.ts`,
compilerOptions: {
...getBaseCompilerOptions(),
// Our normal `tsc` is checking only - but here we want output
noEmit: false,
sourceMap,
},
});
return {
jsCode: result.outputText,
sourceMap: result.sourceMapText ?? null,
};
};

export default compileTypeScript;
2 changes: 1 addition & 1 deletion development/upload-translations.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const run = async () => {
const build = await builder.build();

console.log("Generating strings...");
const l10n = build.generateL10N();
const l10n = await build.generateL10N();

console.log("Uploading runtime strings...");
await uploadRuntimeStrings(l10n["extension-runtime"]);
Expand Down
Loading