edgedb-js icon indicating copy to clipboard operation
edgedb-js copied to clipboard

Improve the type inference performance of `e.op`

Open scotttrinh opened this issue 1 year ago • 2 comments

As written, our current e.op implementation is O(n) on the number of separate overloads we've defined for it, which is currently in the 270ish range. That means overloads define later are more expensive. Recently, TypeScript 5.3 introduced some changes to type inference that have made this bad situation much worse. We need to find a more efficiently set of overloads for e.op that makes selecting the right overload something closer to constant (constant over the number of forms unary, binary, and ternary?) if possible.

scotttrinh avatar May 13 '24 18:05 scotttrinh

I think if you did something like e.op["="](left, right) it doesn't read quite as well but now each op is a separate function (possibly with its own docs and expamples) and you get the discoverability in autocomplete of different ops when you type e.op[".

Might also make sense to add alphanumeric shortcuts like e.op.eq/lt/gt/gte (for =. <. >, >=), then you would be able to avoid the ["..."] syntax most of the time like you could with the in op.

spalger avatar May 13 '24 20:05 spalger

@spalger

We have plans introduce a different and separate API for operators and I'll definitely link here once we get started on that. Thanks for your ideas here, we've toyed around with a similar API and similar symbol vs name concerns, so I know that will be something we address in our updated take on this.

This specific issue is to improve the existing overload API so that the mountains of existing code people have written can benefit from better type inference performance (which recently regressed a ton in TypeScript 5.3) without them having to update their code or writing a code mod, etc.

scotttrinh avatar May 13 '24 20:05 scotttrinh

"Fixed" in #1039

We will more thoroughly address e.op type performance in future work, but this unbreaks the current versions.

scotttrinh avatar Jul 30 '24 14:07 scotttrinh