JST-598: simplify worker type #673
Conversation
BREAKING CHANGE: worker type no longer takes two parameters
src/executor/executor.ts
Outdated
| @@ -117,7 +117,7 @@ export class TaskExecutor { | |||
| private networkService?: NetworkService; | |||
| private statsService: StatsService; | |||
| private activityReadySetupFunctions: Worker[] = []; | |||
| private taskQueue: TaskQueue<Task<unknown, unknown>>; | |||
| private taskQueue: TaskQueue<Task>; | |||
There was a problem hiding this comment.
One nice improvement can be to change the signature to TaskQueue<T extends QueueableTask = Task> to provide a nice default Then we could just use private taskQueue: TaskQueue; instead.
src/golem_network/golem_network.ts
Outdated
| @@ -98,7 +98,7 @@ export class GolemNetwork { | |||
| * console.log(status); | |||
| * ``` | |||
| */ | |||
| public async createJob<Output = unknown>(worker: Worker<unknown, Output>) { | |||
| public async createJob<Output = unknown>(worker: Worker<Output>) { | |||
There was a problem hiding this comment.
I think that we could drop = unknown as the type inference would work here just perfectly.
tests/unit/tasks_service.test.ts
Outdated
| const worker: Worker<null, Result> = async (ctx) => ctx.run("some_shell_command"); | ||
| const task = new Task<null, Result>("1", worker); | ||
| const worker: Worker<Result> = async (ctx) => ctx.run("some_shell_command"); | ||
| const task = new Task<Result>("1", worker); |
There was a problem hiding this comment.
Suggestion: Write code to promote type inference. The equivalent would be:
const worker = async (ctx: WorkContext) => ctx.run("some_shell_command");
const task = new Task("1", worker);This way We don't have to specify Worker<Result> | Task<Result> and the type inference does the job.
tests/unit/work.test.ts
Outdated
| @@ -17,7 +17,7 @@ describe("Work Context", () => { | |||
| describe("Executing", () => { | |||
| it("should execute run command", async () => { | |||
| const activity = await Activity.create("test_agreement_id", yagnaApi); | |||
| const worker: Worker<void, Result> = async (ctx) => ctx.run("some_shell_command"); | |||
| const worker: Worker<Result> = async (ctx) => ctx.run("some_shell_command"); | |||
There was a problem hiding this comment.
Same comment about type inference applies here. Refactored example:
const worker = async (ctx: WorkContext) => ctx.run("some_shell_command");BREAKING CHANGE: `executor.run` always returns the `OutputType` or rejects with an Error. `undefined` has been removed from the return type union.
|
With the change to For reference, previously the type union always had |
|
🎉 This PR is included in version 1.0.0-beta.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
BREAKING CHANGE: worker type no longer takes two parameters