Skip to content

Commit

Permalink
fix(js): format TypeScript files using the faster biomejs formatter a…
Browse files Browse the repository at this point in the history
…nd Google style #1987

This speeds up formatting by 4-12x and fixes several formatting related
issues whether they have to do with the commit message, source code, or
license headers.

```
zsh❯ hyperfine -i "pnpm run format:biome" "pnpm run format:prettier"
Benchmark 1: pnpm run format:biome
  Time (mean ± σ):     453.6 ms ±   5.6 ms    [User: 679.3 ms, System: 92.5 ms]
  Range (min … max):   447.9 ms … 465.3 ms    10 runs

  Warning: Ignoring non-zero exit code.

Benchmark 2: pnpm run format:prettier
  Time (mean ± σ):      5.715 s ±  0.285 s    [User: 9.578 s, System: 0.739 s]
  Range (min … max):    5.506 s …  6.426 s    10 runs

Summary
  pnpm run format:biome ran
   12.60 ± 0.65 times faster than pnpm run format:prettier
```

A sample run follows:

```
zsh❯ pnpm run format

> genkit@ format /Users/yesudeep/code/github.com/firebase/genkit
> pnpm dlx @biomejs/biome format --write . && (tsx scripts/copyright.ts)

Formatted 663 files in 86ms. Fixed 217 files.
Checking copyright in sources...
Updated copyright headers in 0 files
```

The configuration for the formatting is more or less based on the Google
TypeScript formatting guidelines at:

- [ ] https://google.github.io/styleguide/tsguide.html#string-literals
- [ ] https://google.github.io/styleguide/jsguide.html
- [ ] https://google.github.io/styleguide/tsguide.html#arrow-function-bodies
- [ ] https://google.github.io/styleguide/tsguide.html#automatic-semicolon-insertion

ISSUE: #1987

CHANGELOG:
- [ ] Update `pnpm run format` to use biomejs
- [ ] Update `pnpm run format:check` to use biomejs
- [ ] Use `bin/add_license` to add license headers.
- [ ] Exclude the `py/` and `go/` runtimes from the formatter.
- [ ] Fix Import attributes cannot be used with a type-only import error
- [ ] Update all the TypeScript files to use Google-style formatting.
- [ ] Re-enable pre-commit formatting for TypeScript.
  • Loading branch information
yesudeep committed Feb 14, 2025
1 parent 8a40f5b commit 7aab58c
Show file tree
Hide file tree
Showing 359 changed files with 2,184 additions and 2,448 deletions.
1 change: 1 addition & 0 deletions bin/add_license
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ $HOME/go/bin/addlicense \
-ignore '**/dist/**/*' \
-ignore '**/node_modules/**/*' \
-ignore '**/pnpm-lock.yaml' \
-ignore '**/py/site/**/*.html' \
-ignore '.nx/**/*' \
-ignore '.trunk/**/*' \
"$TOP_DIR"
1 change: 1 addition & 0 deletions bin/check_license
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ $HOME/go/bin/addlicense \
-ignore '**/dist/**/*' \
-ignore '**/node_modules/**/*' \
-ignore '**/pnpm-lock.yaml' \
-ignore '**/py/site/**/*.html' \
-ignore '.nx/**/*' \
-ignore '.trunk/**/*' \
"$TOP_DIR"
16 changes: 6 additions & 10 deletions bin/fmt
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,9 @@ fi
popd

# Format all TypeScript code.
#
# TODO: Re-enable once we have biome configured and enabled because that is
# several times faster and compatible.
#
#pushd ${TOP_DIR}
#pnpm run format
#if [[ $? -ne 0 ]]; then
# exit 1
#fi
#popd
pushd ${TOP_DIR}
pnpm run format
if [[ $? -ne 0 ]]; then
exit 1
fi
popd
2 changes: 2 additions & 0 deletions biome.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
".nx/**",
".trunk/**",
"bazel-*/**",
"go/**",
"node_modules/**",
"py/**",
"third_party/**"
]
},
Expand Down
7 changes: 1 addition & 6 deletions genkit-tools/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,7 @@
"version": "1.0.4",
"description": "CLI for interacting with the Google Genkit AI framework",
"license": "Apache-2.0",
"keywords": [
"genkit",
"ai",
"genai",
"generative-ai"
],
"keywords": ["genkit", "ai", "genai", "generative-ai"],
"author": "genkit",
"bin": {
"genkit": "dist/bin/genkit.js"
Expand Down
2 changes: 1 addition & 1 deletion genkit-tools/cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export async function startCLI(): Promise<void> {
// For now only record known command names, to avoid tools plugins causing
// arbitrary text to get recorded. Once we launch tools plugins, we'll have
// to give this more thought
const commandNames = commands.map((c) => c.name());
const commandNames = commands.map(c => c.name());
let commandName: string;
if (commandNames.includes(actionCommand.name())) {
commandName = actionCommand.name();
Expand Down
4 changes: 2 additions & 2 deletions genkit-tools/cli/src/commands/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const CONFIG_TAGS: Record<
string,
(value: string) => string | boolean | number
> = {
[ANALYTICS_OPT_OUT_CONFIG_TAG]: (value) => {
[ANALYTICS_OPT_OUT_CONFIG_TAG]: value => {
let o: boolean | undefined;
try {
o = JSON.parse(value);
Expand All @@ -46,7 +46,7 @@ config
.description('set development environment configuration')
.command('get')
.argument('<tag>', `The config tag to get. One of [${readableTagsHint()}]`)
.action((tag) => {
.action(tag => {
if (!CONFIG_TAGS[tag]) {
logger.error(
`Unknown config tag "${clc.bold(tag)}.\nValid options: ${readableTagsHint()}`
Expand Down
24 changes: 12 additions & 12 deletions genkit-tools/cli/src/commands/eval-extract-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {
import type {
EvalInput,
EvalInputDataset,
TraceData,
Expand Down Expand Up @@ -45,23 +45,23 @@ export const evalExtractData = new Command('eval:extractData')
.option('--maxRows <maxRows>', 'maximum number of rows', '100')
.option('--label [label]', 'label flow run in this batch')
.action(async (flowName: string, options: EvalDatasetOptions) => {
await runWithManager(async (manager) => {
await runWithManager(async manager => {
const extractors = await getEvalExtractors(`/flow/${flowName}`);

logger.info(`Extracting trace data '/flow/${flowName}'...`);
let dataset: EvalInputDataset = [];
let continuationToken = undefined;
while (dataset.length < parseInt(options.maxRows)) {
while (dataset.length < Number.parseInt(options.maxRows)) {
const response = await manager.listTraces({
limit: parseInt(options.maxRows),
limit: Number.parseInt(options.maxRows),
continuationToken,
});
continuationToken = response.continuationToken;
const traces = response.traces;
let batch: EvalInput[] = traces
.map((t) => {
const batch: EvalInput[] = traces
.map(t => {
const rootSpan = Object.values(t.spans).find(
(s) =>
s =>
s.attributes['genkit:metadata:subtype'] === 'flow' &&
(!options.label ||
s.attributes['batchRun'] === options.label) &&
Expand All @@ -73,7 +73,7 @@ export const evalExtractData = new Command('eval:extractData')
return t;
})
.filter((t): t is TraceData => !!t)
.map((trace) => {
.map(trace => {
return {
testCaseId: generateTestCaseId(),
input: extractors.input(trace),
Expand All @@ -82,14 +82,14 @@ export const evalExtractData = new Command('eval:extractData')
// The trace (t) does not contain the traceId, so we have to pull it out of the
// spans, de- dupe, and turn it back into an array.
traceIds: Array.from(
new Set(Object.values(trace.spans).map((span) => span.traceId))
new Set(Object.values(trace.spans).map(span => span.traceId))
),
} as EvalInput;
})
.filter((result): result is EvalInput => !!result);
batch.forEach((d) => dataset.push(d));
if (dataset.length > parseInt(options.maxRows)) {
dataset = dataset.splice(0, parseInt(options.maxRows));
batch.forEach(d => dataset.push(d));
if (dataset.length > Number.parseInt(options.maxRows)) {
dataset = dataset.splice(0, Number.parseInt(options.maxRows));
break;
}
if (!continuationToken) {
Expand Down
14 changes: 7 additions & 7 deletions genkit-tools/cli/src/commands/eval-flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
*/

import {
Action,
Dataset,
DatasetMetadata,
type Action,
type Dataset,
type DatasetMetadata,
DatasetSchema,
} from '@genkit-ai/tools-common';
import {
EvalExporter,
type EvalExporter,
getAllEvaluatorActions,
getDatasetStore,
getExporterForString,
Expand Down Expand Up @@ -83,7 +83,7 @@ export const evalFlow = new Command('eval:flow')
.option('-f, --force', 'Automatically accept all interactive prompts')
.action(
async (flowName: string, data: string, options: EvalFlowRunCliOptions) => {
await runWithManager(async (manager) => {
await runWithManager(async manager => {
const actionRef = `/flow/${flowName}`;
if (!data && !options.input) {
throw new Error(
Expand All @@ -102,7 +102,7 @@ export const evalFlow = new Command('eval:flow')
} else {
const evalActionKeys = options.evaluators
.split(',')
.map((k) => `/evaluator/${k}`);
.map(k => `/evaluator/${k}`);
evaluatorActions = await getMatchingEvaluatorActions(
manager,
evalActionKeys
Expand All @@ -116,7 +116,7 @@ export const evalFlow = new Command('eval:flow')
);
}
logger.debug(
`Using evaluators: ${evaluatorActions.map((action) => action.name).join(',')}`
`Using evaluators: ${evaluatorActions.map(action => action.name).join(',')}`
);

if (!options.force) {
Expand Down
10 changes: 5 additions & 5 deletions genkit-tools/cli/src/commands/eval-run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
* limitations under the License.
*/

import { Action, EvalInputDataset } from '@genkit-ai/tools-common';
import type { Action, EvalInputDataset } from '@genkit-ai/tools-common';
import {
EvalExporter,
type EvalExporter,
getAllEvaluatorActions,
getExporterForString,
getMatchingEvaluatorActions,
Expand Down Expand Up @@ -59,7 +59,7 @@ export const evalRun = new Command('eval:run')
)
.option('--force', 'Automatically accept all interactive prompts')
.action(async (dataset: string, options: EvalRunCliOptions) => {
await runWithManager(async (manager) => {
await runWithManager(async manager => {
if (!dataset) {
throw new Error(
'No input data passed. Specify input data using [data] argument'
Expand All @@ -72,7 +72,7 @@ export const evalRun = new Command('eval:run')
} else {
const evalActionKeys = options.evaluators
.split(',')
.map((k) => `/evaluator/${k}`);
.map(k => `/evaluator/${k}`);
evaluatorActions = await getMatchingEvaluatorActions(
manager,
evalActionKeys
Expand All @@ -86,7 +86,7 @@ export const evalRun = new Command('eval:run')
);
}
logger.info(
`Using evaluators: ${evaluatorActions.map((action) => action.name).join(',')}`
`Using evaluators: ${evaluatorActions.map(action => action.name).join(',')}`
);

if (!options.force) {
Expand Down
6 changes: 3 additions & 3 deletions genkit-tools/cli/src/commands/flow-batch-run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,21 @@ export const flowBatchRun = new Command('flow:batchRun')
fileName: string,
options: FlowBatchRunOptions
) => {
await runWithManager(async (manager) => {
await runWithManager(async manager => {
const inputData = JSON.parse(await readFile(fileName, 'utf8')) as any[];
let input = inputData;
if (inputData.length === 0) {
throw new Error('batch input data must be a non-empty array');
}
if (Object.hasOwn(inputData[0], 'input')) {
// If object has "input" field, use that instead.
input = inputData.map((d) => d.input);
input = inputData.map(d => d.input);
}

const outputValues = [] as { input: any; output: any }[];
for (const data of input) {
logger.info(`Running '/flow/${flowName}'...`);
let response = await manager.runAction({
const response = await manager.runAction({
key: `/flow/${flowName}`,
input: data,
context: options.context ? JSON.parse(options.context) : undefined,
Expand Down
6 changes: 3 additions & 3 deletions genkit-tools/cli/src/commands/flow-run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,17 @@ export const flowRun = new Command('flow:run')
'name of the output file to store the extracted data'
)
.action(async (flowName: string, data: string, options: FlowRunOptions) => {
await runWithManager(async (manager) => {
await runWithManager(async manager => {
logger.info(`Running '/flow/${flowName}' (stream=${options.stream})...`);
let result = (
const result = (
await manager.runAction(
{
key: `/flow/${flowName}`,
input: data ? JSON.parse(data) : undefined,
context: options.context ? JSON.parse(options.context) : undefined,
},
options.stream
? (chunk) => console.log(JSON.stringify(chunk, undefined, ' '))
? chunk => console.log(JSON.stringify(chunk, undefined, ' '))
: undefined
)
).result;
Expand Down
12 changes: 6 additions & 6 deletions genkit-tools/cli/src/commands/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
*/

import {
BaseToolPluginAction,
SpecialAction,
SupportedFlagValues,
ToolPlugin,
type BaseToolPluginAction,
type SpecialAction,
type SupportedFlagValues,
type ToolPlugin,
findToolsConfig,
} from '@genkit-ai/tools-common/plugin';
import { logger } from '@genkit-ai/tools-common/utils';
Expand All @@ -37,8 +37,8 @@ export async function getPluginSubCommand(
): Promise<Command | undefined> {
const config = await findToolsConfig();
const actions = (config?.cliPlugins || [])
.filter((p) => !!p.subCommands?.[commandString])
.map((p) => ({
.filter(p => !!p.subCommands?.[commandString])
.map(p => ({
keyword: p.keyword,
name: p.name,
...p.subCommands![commandString]!,
Expand Down
8 changes: 4 additions & 4 deletions genkit-tools/cli/src/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
* limitations under the License.
*/

import { RuntimeManager } from '@genkit-ai/tools-common/manager';
import { spawn } from 'child_process';
import type { RuntimeManager } from '@genkit-ai/tools-common/manager';
import { startServer } from '@genkit-ai/tools-common/server';
import { logger } from '@genkit-ai/tools-common/utils';
import { spawn } from 'child_process';
import { Command } from 'commander';
import getPort, { makeRange } from 'get-port';
import open from 'open';
Expand Down Expand Up @@ -49,7 +49,7 @@ export const start = new Command('start')
} else {
port = await getPort({ port: makeRange(4000, 4099) });
}
managerPromise = managerPromise.then((manager) => {
managerPromise = managerPromise.then(manager => {
startServer(manager, port);
return manager;
});
Expand Down Expand Up @@ -85,7 +85,7 @@ async function startRuntime(telemetryServerUrl?: string) {
reject(error);
process.exitCode = 1;
});
appProcess.on('exit', (code) => {
appProcess.on('exit', code => {
process.stdin?.pipe(originalStdIn);
if (code === 0) {
urlResolver(undefined);
Expand Down
12 changes: 6 additions & 6 deletions genkit-tools/cli/src/commands/ui-start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@
* limitations under the License.
*/

import { type ChildProcess, spawn } from 'child_process';
import path from 'path';
import {
findServersDir,
logger,
waitUntilHealthy,
} from '@genkit-ai/tools-common/utils';
import axios from 'axios';
import { ChildProcess, spawn } from 'child_process';
import * as clc from 'colorette';
import { Command } from 'commander';
import fs from 'fs/promises';
import getPort, { makeRange } from 'get-port';
import open from 'open';
import path from 'path';

interface StartOptions {
port: string;
Expand Down Expand Up @@ -151,12 +151,12 @@ async function startAndWaitUntilHealthy(
}
);
// Only print out logs from the child process to debug output.
child.on('error', (error) => reject(error));
child.on('exit', (code) =>
child.on('error', error => reject(error));
child.on('exit', code =>
reject(new Error(`UI process exited (code ${code}) unexpectedly`))
);
waitUntilHealthy(`http://localhost:${port}`, 10000 /* 10 seconds */)
.then((isHealthy) => {
.then(isHealthy => {
if (isHealthy) {
child.unref();
resolve(child);
Expand All @@ -169,6 +169,6 @@ async function startAndWaitUntilHealthy(
);
}
})
.catch((error) => reject(error));
.catch(error => reject(error));
});
}
Loading

0 comments on commit 7aab58c

Please sign in to comment.