Skip to content

Type Task.perform as a property - #615

Merged
machty merged 1 commit into
machty:masterfrom
johanrd:bound-perform-type
Sep 24, 2026
Merged

machty merged 1 commit into
machty:masterfrom
johanrd:bound-perform-type

Conversation

@johanrd

@johanrd johanrd commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

perform is bound in the constructor (this.perform = this._perform.bind(this)), but typed as a method. So typescript-eslint's unbound-method reports {{on "click" this.task.perform}} when type-aware rules run on templates (emberjs/rfcs#1045 comment).

Breaking (types only): as a property, perform's args are checked contravariantly, so a Task<void, [string]> no longer fits Task<unknown, unknown[]>. That assignment was unsound: it allowed perform(123) on a task that takes a string. Code that means "any task" can use Task<any, any[]>, which still fits. The type tests pass.

Non-breaking alternative, if you prefer: type the property through a method signature, as @types/react does for event handlers. The args stay bivariant, so assignability does not change, and unbound-method still sees a property:

perform: { bivarianceHack(...args: Args): T }['bivarianceHack'];

Cowritten by Claude

perform is bound at runtime (this.perform = this._perform.bind(this)),
but typed as a method, so typescript-eslint's unbound-method reports
{{on "click" this.task.perform}}.

As a property, its args are checked contravariantly: a Task<T, [string]>
no longer fits Task<unknown, unknown[]>. Task<any, any[]> still fits.
@machty

machty commented Sep 23, 2026

Copy link
Copy Markdown
Owner

I don't know what "contravariantly" means, mainly I just want to know "is this likely to bite anyone" for the 99% use caes of ember-concurrency, then I'll merge.

@johanrd

johanrd commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

First: it only affects TypeScript compilation, never runtime, and only code that stores a task in a type that accepts any arguments, like Task<unknown, unknown[]>.

I asked claude to find public TypeScript projects that use ember-concurrency (boxel, ember-power-select, hashicorp/design-system, ember-bootstrap, …), and of 13, 12 are unaffected. One, eclipse-pass/pass-ui, uses Task<unknown, unknown[]> as "any task" in 5 places. passing task(async (page: number) => …) there would become a type error. Their fix is one word: Task<unknown, any[]>, which honestly is more accurate: unknown[] never checked the args anyway, since the method type let any task in and allowed perform(123) on a task that takes a string.

So 99% is maybe a stretch. It can break compilation, so maybe bump as a major? (integers are cheap, etc)

@machty

machty commented Sep 24, 2026

Copy link
Copy Markdown
Owner

I think safe to release as minor. In case major issues i'll revert and publish a new minor and then push major with the fix.

@machty
machty merged commit 507e902 into machty:master Sep 24, 2026
17 checks passed
@machty

machty commented Sep 24, 2026

Copy link
Copy Markdown
Owner

https://github.com/machty/ember-concurrency/releases/tag/5.3.0

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