-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More specific TemplateStringsArray type for tagged templates #49552
base: main
Are you sure you want to change the base?
Conversation
a8bafb5
to
883fc2a
Compare
src/compiler/checker.ts
Outdated
let constituents: Type[] | undefined; | ||
for (const constituent of (type as IntersectionType).types) { | ||
if (isArrayOrTupleType(constituent)) { | ||
arrayOrTuple = constituent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this always just picks the last array or tuple type, making it order-dependent which tuple is picked, which seems bad. We should either have tuple-intersecting behavior or return undefined
when there are multiple arrays/tuples, IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are already order dependent for this case: Playground link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think intersecting the tuples is viable here, as it introduces far too much complexity. I'm leaning towards what appears to be the current behavior in this case (i.e., picking the right-most), though another viable alternative would be to pick neither if there is more than one (since they would overlap).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Picking neither if there's more than one is often what we do in other situations, and order-dependence in inference is bad anywhere it crops up, since it means inference results can shift with file ordering, which is a pain in the butt bug to track down when it's reported later. Our existing left-preferring behavior among multiple matching (mutually subtyped) inferences is already a big "this really should be fixed"; I'd much prefer to bring the cases that introduce order dependence down if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like most of this change, it's not too big, either; but there's some intersection-order-dependence in the inference code I think we should remove, and a debugger
statement that obviously needs to go. 😄
883fc2a
to
70fdb52
Compare
I tinkered with an example of a tail-recursive parser for |
Here's a much simpler version of Jest's type IDStart =
| "a" | "b" | "c" | "d" | "e" | "f" | "g" | "h" | "i" | "j" | "k" | "l" | "m" | "n" | "o" | "p" | "q" | "r" | "s" | "t" | "u" | "v" | "w" | "x" | "y" | "z"
| "A" | "B" | "C" | "D" | "E" | "F" | "G" | "H" | "I" | "J" | "K" | "L" | "M" | "N" | "O" | "P" | "Q" | "R" | "S" | "T" | "U" | "V" | "W" | "X" | "Y" | "Z"
| "$" | "_"
;
type IDPart =
| "0" | "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9"
| IDStart
;
type Trim<S extends string, Chars extends string = " " | "\t" | "\v" | "\n"> =
S extends (Chars extends "" ? S : never) ? S :
S extends `${Chars}${infer R}` ? Trim<R, Chars> :
S extends `${infer R}${Chars}` ? Trim<R, Chars> :
S;
type ParseHeaders<S extends string> =
S extends "|" ? [] :
S extends `${infer H}|${infer T}` ? [Trim<H>, ...ParseHeaders<T>] :
[Trim<S>];
type CheckHeader<TIn extends string, TWork extends string = TIn, Valid extends IDPart = IDStart> =
TWork extends `${infer Ch}${infer R}` ?
Ch extends Valid ? CheckHeader<TIn, R, IDPart> :
JestEachParseError<`Invalid identifier: '${TIn}'`> :
TIn extends '' ? JestEachParseError<`Identifier expected.`> :
TIn;
type CheckHeaders<TIn extends string[], TWork extends string[] = TIn> =
TWork extends [infer TH extends string, ...infer TT extends string[]] ?
CheckHeader<TH> extends infer E extends JestEachParseError<any> ? E :
CheckHeaders<TIn, TT> :
TIn;
type ParseRows<A extends any[], S extends readonly string[], Row extends any[] = [], Rows extends any[][] = []> =
[A, S] extends [[infer AH, ...infer AT], readonly [infer TH extends string, ...infer TT extends string[]]] ?
Trim<TH, " " | "\t" | "\v"> extends "|" ? ParseRows<AT, TT, [...Row, AH], Rows> :
Trim<TH, " " | "\t" | "\v"> extends "\n" | "" ? ParseRows<AT, TT, [], [...Rows, [...Row, AH]]> :
JestEachParseError<`Unexpected character: '${Trim<TH>}'`> :
[A, S] extends [[], readonly []] ? Rows :
JestEachParseError<`Mismatched elements`>;
type JestEachArgument<Headers extends string[] | JestEachParseError<any>, Rows extends any[][] | JestEachParseError<any>> =
Headers extends string[] ?
Rows extends any[][] ?
{
[P1 in keyof Rows]: {
[P2 in keyof Headers as P2 extends `${number}` ? Headers[P2] : never]:
P2 extends keyof Rows[P1] ? Rows[P1][P2] : undefined;
};
}[number] :
Rows :
Headers;
type JestEachFunction<Arg> =
Arg extends JestEachParseError<any> ? Arg :
(name: string, cb: (arg: Arg) => void, timeout?: number) => void;
declare const JestEachParseError: unique symbol;
interface JestEachParseError<Message extends string> {
[JestEachParseError]: Message;
}
type JestEach<T extends readonly string[], A extends any[]> =
T extends readonly [infer TH extends string, ...infer TT extends readonly string[]] ?
JestEachFunction<JestEachArgument<CheckHeaders<ParseHeaders<TH>>, ParseRows<A, TT>>> :
JestEachParseError<`Mismatched elements`>;
declare function each<T extends readonly string[], A extends (string | symbol | number | bigint | boolean | null | undefined | object)[]>(strs: T, ...args: A): JestEach<T, A>;
// jest
it.each`
foo | bar
${"a"} | ${"b"}
${"a"} | ${"c"}
`("test", ({ foo, bar }) => {
// ^? any
// ^? any
});
each`
foo | bar
${"a"} | ${"b"}
${"c"} | ${"d"}
`("test", ({ foo, bar }) => {
// ^? "a" | "c"
// ^? "b" | "d"
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer the order dependence removed from inference, and I'd love to see that .each
example in the test baselines (it probably exercises a bunch that we have minimal coverage of in concert, and would be useful to know we don't regress it), but I'm willing to approve this now, even if I'd like to see a bit more.
1e45293
to
f8752ad
Compare
There is no longer any order dependence. The behavior for inferring from an intersection of two tuple types now falls back to pre-existing behavior. |
Also, I've added a test for Jest's |
@DanielRosenwasser: Are we comfortable taking this for 4.8 or would you rather wait? |
@typescript-bot perf test |
Heya @rbuckton, I've started to run the diff-based user code test suite on this PR at f8752ad. You can monitor the build here. Update: The results are in! |
@rbuckton |
@typescript-bot perf test |
Heya @rbuckton, I've started to run the perf test suite on this PR at e079811. You can monitor the build here. Update: The results are in! |
@rbuckton Here they are:Comparison Report - main..49552
System
Hosts
Scenarios
Developer Information: |
This is a feature I've been waiting for a long time, but I forgot why. 😄 |
Hi, this PR looks fantastic (I've been hoping for this feature for a long time)! Wondering what the status is? |
Is there any progress for this feature? |
I hope the pending reviews can be done soon so we will see this released soon. This would open the door to a lot of interesting things. |
This PR is currently on hold. The main use cases for this feature require a fair amount of complexity that is likely to run any implementation against the recursion depth limiter. Such types are expensive to compute and check, and often result in degraded performance in an editor. It's unlikely that we will ship a feature that introduces this much complexity without first making significant improvements in overall type checking performance. I do plan to keep this PR alive and will periodically update it, but I do not foresee it shipping in the near future. |
There are also use cases which require no recursions. The idea is to generate return types for queries based on your written code. import { gql } from "gql.generated"; // <--- gqls types are generated from all .ts files
const userQuery = const USER_QUERY = gql(`
query User($id: String!) {
user(id: $id) {
id
name
}
}
`);
execute(userQuery, "12") // <--- typed - will return Promise<{user: { id: string, name: string } }> However for graphql fragments we would need to use template literals to merge the return types of the query and fragments: import { gql } from "gql.generated";
const fragment = gql`a fragment query`;
const userQuery = gql`a graphql query { ${fragment} }` |
Another year has passed, and version 5.x has now been released. This version has optimized the performance of comparison. Is it possible to consider introducing this feature in the new version? This helps us a lot, thank you very much for your work. |
i wrote a working type level parser for my library's html template string DSL so it can be typesafe but it's useless without this PR. JSX isn't an option due to lack of ability to cache static parts with default tooling. i'd love to see this merged. |
@rbuckton Any updates? Aren't v5's perf enhancements sufficient now? |
@typescript-bot pack this |
Starting jobs; this comment will be updated as builds start and complete.
|
Hey @rbuckton, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build and an npm module you can use via |
No, unfortunately. The performance concerns we have are related to the complex conditional types and inference that are necessary to "parse" a string literal type, and that still isn't very efficient at the moment. |
@rbuckton Has ArkType not proven it can be done efficiently at this point? Even with the overhead of type-level string parsing (including syntactic and semantic validation), types are often 3-10x less expensive than equivalent Zod. It would be a shame not to ship what would be an awesome feature just because it is possible to use inefficiently. |
We need this merged yesterday ❤️ Typescript users are harassing me into changing my library APIs to be different just because they can't do what this PR would allow them. Why not let the resposibility of making fast implementations, and avoiding slow ones be up to the users? |
FWIW, @jantimon's use case, as well as a similar use case I'm working on regarding SQL queries, do not make use of conditional types or literal type parsing. If I understand him correctly, the use of literal types we have in mind boils down to using external tools to generate overloads from user code, like so: function sql(strings: readonly ["select ", " + 1"], parameter: number): number;
function sql(strings: readonly ["select ", " || ' world!'"], parameter: string): string;
... No literal parsing required. I'd argue that tools like this will be the main use cases in the near future, since users will avoid libraries that do complex parsing within TypeScript because they are too slow. |
What about those of us who just wants a way to map types against a tagged template literal function starting with specific words? |
It sounds like that would be supported by this PR. And simple enough that it wouldn't run into TypeScript performance constraints, and so, under my theory, be adopted widely by users and be one of "the main use cases", unlike more complex use cases that may need to wait for a more performant TypeScript to gain adoption. My point is that performance is an important part of DX, and this is something that library authors and users already have to take into account. For example, There are plenty of use cases for this feature that improve DX and reliability without sacrificing editor performance by limiting the amount of work done in the type checker. Is there anything would-be-users of this feature could do to help move it along? |
Is it possible to release this pr with 5.7? |
This introduces a more specific type over
TemplateStingsArray
when invoking a tagged template. This would then allow developers to interpolate the literal types provided to the tagged template invocation, along with the provided arguments:The type we manufacture for the tagged template invocation is an instantiation of
TemplateStringsArrayOf<Cooked, Raw>
:This would then allow you to perform custom interpolation against either the "cooked" values (where escapes are normalized), and "raw" values (where escapes are preserved):
In theory, more complex interpolation could also be achieved, but that is an exercise left to the reader:
To support interpolation over a
TemplateStringsArray
, I also needed to make a change toinferFromObjectTypes
. Prior to this change you could not infer to tuple element positions if the source type was an intersection of a tuple and another type:However, this becomes a problem when trying to pick apart a generic
TemplateStringsArrayOf
type. To address this, this PR makes a small change in how we perform this inference. Nothing changes when a source type is already an array or tuple type, but if the source type is instead an intersection of an array or tuple type and another type, we will also now permit tuple element inference as long as the intersected types do not shadow tuple-specific properties (i.e., numeric indexers, numeric-stringed properties, or the"length"
property):Fixes #33304
Fixes #31422
Related #29432
Related #16551