Skip to content
Merged
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
5 changes: 5 additions & 0 deletions node-src/lib/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ export const buildDeps = (ctx: Context): Deps => ({
pkg: ctx.pkg,
sessionId: ctx.sessionId,
packageJson: ctx.packageJson,
// No-op by default; the new engine's `runTask` overrides this with a renderer-aware reporter.
// Under Listr there is no reporter: legacy tasks mutate the Listr task object directly, and
// adapted tasks have no mid-task channel here — so a task must not adopt `deps.report` until it
// has been migrated off Listr onto `runTask`, where the override makes it live.
report: () => {},
});

async function applyAdaptedResult<TInput, TOutput, TPartial>(
Expand Down
107 changes: 107 additions & 0 deletions node-src/renderer/engine/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,111 @@ describe('runTask', () => {
const fail = calls.find((c) => c.hook === 'fail');
expect(fail?.state.title).toBe('Authentication failed');
});

describe('deps.report (mid-task updates)', () => {
it('routes a textual update to renderer.update without calling onTaskProgress', async () => {
const onTaskProgress = vi.fn();
const ctx = fakeContext({ options: { experimental_onTaskProgress: onTaskProgress } as any });
const { renderer, calls } = recordingRenderer();

const config: TaskConfig<unknown, unknown> = {
name: 'gitInfo',
title: 'Retrieving git information',
transitions,
extractInput: () => ({}),
run: async (deps) => {
deps.report({ title: 'Skipping build', output: 'no changes since last build' });
return { kind: 'continue', output: {} };
},
};

await runTask(ctx, config, renderer);

const update = calls.find((c) => c.hook === 'update');
expect(update?.state.title).toBe('Skipping build');
expect(update?.state.output).toBe('no changes since last build');
expect(update?.state.progress).toBeUndefined();
expect(onTaskProgress).not.toHaveBeenCalled();
});

it('fans a numeric update out to both renderer.update and the public onTaskProgress hook', async () => {
const onTaskProgress = vi.fn();
const ctx = fakeContext({ options: { experimental_onTaskProgress: onTaskProgress } as any });
const { renderer, calls } = recordingRenderer();

const config: TaskConfig<unknown, unknown> = {
name: 'upload',
title: 'Uploading',
transitions,
extractInput: () => ({}),
run: async (deps) => {
deps.report({
output: 'uploading',
progress: { progress: 512, total: 1024, unit: 'bytes' },
});
return { kind: 'continue', output: {} };
},
};

await runTask(ctx, config, renderer);

const update = calls.find((c) => c.hook === 'update');
expect(update?.state.progress).toEqual({ progress: 512, total: 1024, unit: 'bytes' });
// No textual report preceded this one, so the update inherits the task's starting title.
expect(update?.state.title).toBe('Uploading');

expect(onTaskProgress).toHaveBeenCalledExactlyOnceWith(expect.anything(), {
progress: 512,
total: 1024,
unit: 'bytes',
});
});

it('passes a numeric-only report through with no output', async () => {
const ctx = fakeContext({ options: {} as any });
const { renderer, calls } = recordingRenderer();

const config: TaskConfig<unknown, unknown> = {
name: 'upload',
title: 'Uploading',
transitions,
extractInput: () => ({}),
run: async (deps) => {
deps.report({ progress: { progress: 512, total: 1024, unit: 'bytes' } });
return { kind: 'continue', output: {} };
},
};

await runTask(ctx, config, renderer);

const update = calls.find((c) => c.hook === 'update');
expect(update?.state.output).toBeUndefined();
expect(update?.state.progress).toEqual({ progress: 512, total: 1024, unit: 'bytes' });
});

it('makes a textual update sticky: a later title-less report inherits the current title', async () => {
const ctx = fakeContext({ options: {} as any });
const { renderer, calls } = recordingRenderer();

const config: TaskConfig<unknown, unknown> = {
name: 'upload',
title: 'Uploading',
transitions,
extractInput: () => ({}),
run: async (deps) => {
deps.report({ title: 'Finalizing' });
deps.report({ progress: { progress: 512, total: 1024, unit: 'bytes' } });
return { kind: 'continue', output: {} };
},
};

await runTask(ctx, config, renderer);

const updates = calls.filter((c) => c.hook === 'update');
expect(updates[0].state.title).toBe('Finalizing');
// The numeric-only report carries no title of its own, so it renders with the title set by
// the preceding textual report.
expect(updates[1].state.title).toBe('Finalizing');
});
});
});
36 changes: 33 additions & 3 deletions node-src/renderer/engine/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// alone for now to avoid a noisy diff.
import { buildDeps } from '@cli/tasks';

import { Context, Deps, Task, TaskFunction, TaskName, TaskResult } from '../../types';
import { Context, Deps, Task, TaskFunction, TaskName, TaskReporter, TaskResult } from '../../types';

/**
* A `TaskRenderer` consumes the `Task` state objects emitted by a task's
Expand All @@ -15,7 +15,10 @@ import { Context, Deps, Task, TaskFunction, TaskName, TaskResult } from '../../t
export interface TaskRenderer {
/** Task has begun. `state` is the `pending` transition. */
start: (state: Task) => void;
/** Progress update while the task body runs. `state.output` contains the message. */
/**
* Progress update while the task body runs, pushed via `deps.report`. `state.output` carries the
* message; `state.progress` carries optional numeric progress for renderers that draw a bar.
*/
update: (state: Task) => void;
/** Task finished successfully. `state` is the `success` or `partial` transition. */
succeed: (state: Task) => void;
Expand Down Expand Up @@ -70,7 +73,8 @@ export async function runTask<TInput, TOutput, TPartial = never>(

try {
const input = config.extractInput(ctx);
const result = await config.run(buildDeps(ctx), input);
const deps = { ...buildDeps(ctx), report: makeReporter(ctx, renderer) };
const result = await config.run(deps, input);
await applyResult(ctx, config, result, renderer);
} catch (error) {
renderer.fail(failureState(ctx, config, error as Error, pending.title));
Expand All @@ -80,6 +84,32 @@ export async function runTask<TInput, TOutput, TPartial = never>(
ctx.options.experimental_onTaskComplete?.({ ...ctx });
}

/**
* Build the reporter injected as `deps.report`, used to enable mid-task UI updates through the
* renderer's `update` hook. These updates are also fanned out to `experimental_onTaskProgress`
* (this callback only accepts numeric progress, not textual updates).
*
* @param ctx The CLI context (its `title` is updated when a textual update arrives).
* @param renderer The active renderer for the running task.
*
* @returns A `TaskReporter` closed over the current task's renderer.
*/
function makeReporter(ctx: Context, renderer: TaskRenderer): TaskReporter {
return (update) => {
if (update.title !== undefined) ctx.title = update.title;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Personally, I'm not a big fan of single-line if statements, but I won't hold up review for this. Also, this could be simplified to if (update.title), unless there's some explicit reason you want to test for not undefined.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's just to match Listr semantics. Our previous tasks would update the ctx.title in this way too (i.e., allowing the empty string). I'm not sure it will ever be used anywhere, but the individual renderers can decide what to do with the information.


renderer.update({
title: ctx.title,
output: update.output,
progress: update.progress,
});

if (update.progress) {
ctx.options.experimental_onTaskProgress?.({ ...ctx }, update.progress);
}
};
}

/**
* Maps task results to context mutations and UI state objects, as well as calling the underlying renderer to
* drive the UI.
Expand Down
36 changes: 32 additions & 4 deletions node-src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,7 @@ export interface Options extends Configuration {
) => void;

/** A callback that is called during tasks that have incremental progress */
experimental_onTaskProgress?: (
ctx: Context,
status: { progress: number; total: number; unit: string }
) => void;
experimental_onTaskProgress?: (ctx: Context, status: TaskProgress) => void;

/** A callback that is called at the completion of each task */
experimental_onTaskComplete?: (ctx: Context) => void;
Expand Down Expand Up @@ -291,6 +288,12 @@ export interface Deps {
pkg: Context['pkg'];
sessionId: string;
packageJson: Record<string, any>;
/**
* Push a mid-task UI update from inside the task body. The base `buildDeps` provides a no-op;
* `runTask` overrides it with a renderer-aware reporter that fans out to the active renderer and
* the public `experimental_onTaskProgress` hook.
*/
report: TaskReporter;
}

export type TaskResult<TOutput, TPartial = never> =
Expand Down Expand Up @@ -462,8 +465,33 @@ export interface Task {
status?: string;
title: string;
output?: string;
/**
* Optional numeric progress. Renderers that can draw a progress bar (e.g. the Clack progress-bar
* renderer) consume this; renderers that can't simply ignore it and render the text.
*/
progress?: TaskProgress;
}

export interface TaskProgress {
progress: number;
total: number;
unit: string;
}

/**
* A mid-task update pushed from inside a task body via `deps.report`. This is the channel for any
* UI change that happens *during* the business logic (a textual phase change like gitInfo's
* "skipping build", or numeric progress like upload bytes / snapshot counts) — as opposed to the
* `start`/`succeed`/`fail` brackets the engine drives on its own.
*/
export interface TaskUpdate {
title?: string;
output?: string;
progress?: TaskProgress;
}

export type TaskReporter = (update: TaskUpdate) => void;

export interface Reason {
moduleName: string;
}
Expand Down
Loading