GeezerVison

TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) test top100
TypeScript Bot (@typescript-bot) user test this
TypeScript Bot (@typescript-bot) run dt
TypeScript Bot (@typescript-bot) perf test this faster
TypeScript Bot (@typescript-bot) pack 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
TypeScript Bot (@typescript-bot) test top100
TypeScript Bot (@typescript-bot) user test this
TypeScript Bot (@typescript-bot) run dt
TypeScript Bot (@typescript-bot) perf test this faster
TypeScript Bot (@typescript-bot) pack 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 package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/153951/artifacts?artifactName=tgz&fileId=DE5A96061A3950803531AF84D9DFCC089114B6A0673013CE79EEC4F7D4BA0BE902&fileName=/typescript-5.1.0-insiders.20230505.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

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 const x = `${"a"}` having type string and not type "a". Let's see when the tests finish running, I guess.

Gabriela Araujo Britto (@gabritto) Here are the results of running the user test suite comparing main and refs/pull/53907/merge:

There were infrastructure failures potentially unrelated to your change:

• 1 instance of "Unknown failure"
• 1 instance of "Package install failed"

Otherwise...

Everything looks good!

Gabriela Araujo Britto (@gabritto) Here are the results of running the top-repos suite comparing main and refs/pull/53907/merge:

Everything looks good!

Hey Gabriela Araujo Britto (@gabritto), the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

Gabriela Araujo Britto (@gabritto)
The results of the perf run you requested are in!

Wesley Wigham (@weswigham) do we have concerns with merging this?

Sorry, how is this different from #41891 which we had to back out?

Refs:

• Design Meeting Notes, 1/15/2021 #42416
• Design Meeting Notes, 1/22/2021 #42589
• Template literal types for contextually typed template literal expressions #43376

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 undefined and every code that follows that continues to work as before.

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 ok

And 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 thing

There is also an argument about makePx - that it would behave differently. This is true but then the argument wouldn't be generic so the input type wouldn't be preserved. Even if we make it generic - the return type would still be string since the generic signature can't determine how it will evaluate and if literal types are the only inputs given to this function.

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.