Evaluate simple template expressions #53907
Conversation
|
@typescript-bot test this |
|
I think in theory this is a good change (assuming the tests come back passing), the question is if it's worth the perf cost. |
|
@typescript-bot test this |
|
Heya @gabritto, I've started to run the parallelized Definitely Typed test suite on this PR at 02b5df0. You can monitor the build here. Update: The results are in! |
|
Heya @gabritto, I've started to run the diff-based top-repos suite on this PR at 02b5df0. You can monitor the build here. Update: The results are in! |
|
Heya @gabritto, I've started to run the diff-based user code test suite on this PR at 02b5df0. You can monitor the build here. Update: The results are in! |
|
Heya @gabritto, I've started to run the abridged perf test suite on this PR at 02b5df0. You can monitor the build here. Update: The results are in! |
|
Hey @gabritto, 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 |
|
I'm now also wondering if someone might be relying on things like |
|
@gabritto Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
|
@gabritto Here are the results of running the top-repos suite comparing Everything looks good! |
|
Hey @gabritto, the results of running the DT tests are ready. |
|
@gabritto Here they are:Comparison Report - main..53907
System
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@weswigham do we have concerns with merging this? |
|
Sorry, how is this different from #41891 which we had to back out? Refs: |
|
This PR is much smaller in scope. It doesn't infer template literal types for template literal expressions. Only string literals can be inferred from template literal expressions. If the evaluation ends up being impossible then evaluation just returns I looked through all the linked issues/PRs and I think that most of the concerns raised there were about template literal types. For example, second bullet point from the bottom in #42416: const x = `somestring`; // this is a literal type
const str = "string";
const y = `some${str}`; // and this is too! (but wasn't)Inferring a template literal type here wouldn't be OK since it's not uncommon to push new items to the resulting array: declare var srcSvgData: SvgData[];
interface SvgData {
fileName: string;
iconName: string;
}
const c = srcSvgData.map(
(svgData) =>
`<a href="./svg/${svgData.fileName}"><svg><use href="#${svgData.iconName}" xlink:href="#${svgData.iconName}"/></svg></a>`
);
// https://github.com/ionic-team/ionicons/blob/1543daa0103c9065805f7075b5dc6abd1c2dd9c1/scripts/build.ts#L262
c.push("somestring"); // should be okAnd the last one: function usePxValue(px: `${string}px`) {}
let a = 12;
const lit = `${a}px`;
usePxValue(lit); // error, this makes sense though since `a` is mutable binding with a `number` type
const b = 12;
const lit2 = `${b}px`;
usePxValue(lit2); // this works though, it seems like a good thingThere is also an argument about This works though since the programmer encoded the specific intent for the return type and the argument is generic: function makePx1<T extends string>(a: T) {
return `${a}px` as const;
}Maybe there is an argument to be made here that constness could propagate to this return expression automatically: function makePx2<const T extends string>(a: T) {
return `${a}px`; // this isn't const
}I'm not precluding that there might be some edge cases here around widening but I don't know all of its intricate/delicate rules so it's hard for me to comment on that. |
fixes #53888
This PR builds on top of the simple fix available in #53903 . Since I think that it's not that unlikely that this other PR might be desirable while this one here might not be, I decided to open an extra PR with this change.
I don't think that the case presented in #53888 warrants the change but I can imagine that it's a simplification for the purpose of creating a minimal repro case. A more sane case could look smth like this:
Given that we are able to "evaluate" some constant values this PR would allow for simple variable reuse and composition of template expressions without losing the ability to discriminate objects etc