Evaluate simple template expressions - #53907
Conversation
TypeScript Bot (typescript-bot)
added
the
For Backlog Bug
|
TypeScript Bot (@typescript-bot) test this |
|
Heya Gabriela Araujo Britto (@gabritto), I'm starting to run the abridged perf test suite on this PR at 02b5df0. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
Heya Gabriela Araujo Britto (@gabritto), I'm starting to run the tarball bundle task on this PR at 02b5df0. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
Heya Gabriela Araujo Britto (@gabritto), I'm starting to run the parallelized Definitely Typed test suite on this PR at 02b5df0. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
Heya Gabriela Araujo Britto (@gabritto), I'm starting to run the diff-based user code test suite on this PR at 02b5df0. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
Heya Gabriela Araujo Britto (@gabritto), I'm starting to run the diff-based top-repos suite on this PR at 02b5df0. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
Heya Gabriela Araujo Britto (@gabritto), I'm starting to run the extended test suite on this PR at 02b5df0. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
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 (@typescript-bot) test this |
|
Heya Gabriela Araujo Britto (@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 Gabriela Araujo Britto (@gabritto), I've started to run the tarball bundle task on this PR at 02b5df0. You can monitor the build here. |
|
Heya Gabriela Araujo Britto (@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 Gabriela Araujo Britto (@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 Gabriela Araujo Britto (@gabritto), I've started to run the extended test suite on this PR at 02b5df0. You can monitor the build here. |
|
Heya Gabriela Araujo Britto (@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 Gabriela Araujo Britto (@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 |
|
Heya Gabriela Araujo Britto (@gabritto), I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
|
I'm now also wondering if someone might be relying on things like |
|
Gabriela Araujo Britto (@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! |
|
Gabriela Araujo Britto (@gabritto) Here are the results of running the top-repos suite comparing Everything looks good! |
|
Hey Gabriela Araujo Britto (@gabritto), the results of running the DT tests are ready. |
|
Gabriela Araujo Britto (@gabritto) Here they are:Comparison Report - main..53907
System
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Wesley Wigham (@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. |
6 participants
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