Skip to content

Conversation

@danielpza
Copy link
Contributor

@danielpza danielpza commented Jun 3, 2025

This is technically a breaking change in the typescript api.

Changed the generics from

const pool = new Piscina<Parameters<typeof import("./worker.js").default>[0], ReturnType<typeof import("./worker.js").default>>({ filename: "./worker.js" })

// or specified manually
const pool = new Piscina<{a: number, b: number}, number>({ filename: "./worker.js" })

to

const pool = new Piscina<typeof import("./worker.js")>({ filename: "./worker.js" })

// or specified manually
const pool = new Piscina<{default: (payload: { a: number, b: number }) => number}>({ filename: "./worker.js" })

Also works with multiple exports:

// worker.ts
export function greet({ name }: { name: string }): string {
  return `Hello ${name}!`;
}

export default function add({ a, b }: { a: number; b: number }): number {
  return a + b;
}
// main.ts
const pool = new Piscina<typeof import("./worker.js")>({ filename: "./worker.js" })

pool.run({
  // types are checked for default export
}) // returns Promise<number>

pool.run({
  // types are checked for greet export
}, { name: 'greet' }) // returns Promise<string>

This (imo) improves the typescript usage by reducing the amount of code needed to properly type the worker function.

@danielpza danielpza force-pushed the typescript-entrypoint branch from 65fc6db to eae9a29 Compare June 3, 2025 22:06
@danielpza danielpza marked this pull request as ready for review June 3, 2025 22:07
@danielpza danielpza force-pushed the typescript-entrypoint branch 2 times, most recently from a343e5e to 143d0d3 Compare June 3, 2025 23:34
Copy link
Member

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

I really like this, but sadly it imposes a breaking change in the current version; can we aim to add a fallback to the current behavior if the generic passed is not an object?

@danielpza
Copy link
Contributor Author

I really like this, but sadly it imposes a breaking change in the current version; can we aim to add a fallback to the current behavior if the generic passed is not an object?

I played around with the code a bit to see if it was possible, unfortunately I think it's not possible (or I don't know enough TS) to make it backwards compatible.

@metcoder95
Copy link
Member

I'm thinking on function overloads, but not sure if they can be applied to classes 🤔

Otherwise, we can point this feature directly to v6, but will take some time to land as it will be aimed to next year.

@danielpza
Copy link
Contributor Author

danielpza commented Jun 5, 2025

I'm thinking on function overloads, but not sure if they can be applied to classes 🤔

I thought the same but couldn't get it to work, seems it's not possible.

Otherwise, we can point this feature directly to v6, but will take some time to land as it will be aimed to next year.

I'm fine with this going to v6.

If someone later finds a way to backport it without breaking compatibility that would be great.

@metcoder95
Copy link
Member

Perfect, then let's do this. I'll sync next branch to contain latest changes and we point this PR to that branch instead.

If willing, we can give it a try to backport it aftewards, otherwise we can open an Issue for someone that might be interested in contribute

@metcoder95 metcoder95 changed the base branch from current to v6 June 6, 2025 08:03
@metcoder95 metcoder95 changed the base branch from v6 to current June 8, 2025 09:15
@github-actions
Copy link

This issue has been marked as stale because it has been opened 45 days without activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale label Jul 13, 2025
@danielpza danielpza force-pushed the typescript-entrypoint branch from 143d0d3 to 234d5b9 Compare July 19, 2025 01:29
@danielpza
Copy link
Contributor Author

@metcoder95 rebased to the current branch. Can you approve the workflows run again?

@metcoder95
Copy link
Member

CI failures on Windows are expected; not related to you PR

@github-actions github-actions bot removed the stale label Jul 20, 2025
@danielpza danielpza force-pushed the typescript-entrypoint branch from 217acd4 to 6cf8f01 Compare July 23, 2025 07:34
@danielpza danielpza requested a review from metcoder95 July 23, 2025 15:40
@metcoder95
Copy link
Member

Test seems to fail :(

@danielpza
Copy link
Contributor Author

Test seems to fail :(

Pushed another commit, this should fix it now.

@metcoder95
Copy link
Member

Seems like it still doesn't like it

@danielpza
Copy link
Contributor Author

danielpza commented Jul 27, 2025

Seems like it still doesn't like it

I messed up the steps order, I fixed it, it should be good now.

@metcoder95 metcoder95 merged commit 1a1dbed into piscinajs:current Jul 28, 2025
10 of 12 checks passed
@metcoder95
Copy link
Member

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants