Skip to content
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

feat(33304): Better type-safety with tagged template literals #45310

Closed
wants to merge 13 commits into from

Conversation

BobobUnicorn
Copy link

This PR implements better type-checking for tagged template literals. The main change is that TemplateStringsArray is now generic over its string parts. In particular, TemplateStringsArray is now:

type TemplateStringsArray<T extends readonly string[] = readonly string[]> = T & {
    readonly raw: T;
};

The type parameter has a default for backwards-compatibility.

This PR also includes compiler changes to pass the exact string parts as a tuple for inference.

Fixes #33304

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Aug 3, 2021
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #33304. If you can get it accepted, this PR will have a better chance of being reviewed.

@ghost
Copy link

ghost commented Aug 3, 2021

CLA assistant check
All CLA requirements met.

@BobobUnicorn BobobUnicorn changed the title Better type-safety with tagged template literals feat(33304): Better type-safety with tagged template literals Aug 3, 2021
@tjjfvi
Copy link
Contributor

tjjfvi commented Aug 3, 2021

raw should probably be typed as just string[], as it's not always identical; (x=>x.raw)`ab\c`​ is ["ab\\c"].

@BobobUnicorn
Copy link
Author

Ah, interesting. That should be a simple change.

@DanielRosenwasser DanielRosenwasser added this to Agenda in Design Meeting Docket via automation Aug 11, 2021
@sandersn sandersn added this to Not started in PR Backlog Aug 13, 2021
}

var a = tag `part\1${''}part\2`;
var a: {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this here?


declare function dataTypes<TParts extends readonly string[], TArgs extends ArgsFor<TParts>>(
parts: TemplateStringsArray<readonly [...TParts, '']>,
...args: TArgs): unknown;
Copy link
Member

@DanielRosenwasser DanielRosenwasser Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this return TParts so we can witness the inference result?

@DanielRosenwasser
Copy link
Member

Personally I'm hesitant to add more complexity to TemplateStringsArray - especially since it's inevitable that people will overuse this to write slow DSL parsers on long template strings.

That said, it does feel like there should be a way to capture the contents of a template string today.

@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 18, 2021

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 90345f8. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 18, 2021

Hey @DanielRosenwasser, 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/108904/artifacts?artifactName=tgz&fileId=6D928E65AE8E1AD1EC006AE31607AE0F05AA5CA4EF31E510B71DADA9C5F16A9D02&fileName=/typescript-4.5.0-insiders.20210818.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/pr-build@4.5.0-pr-45310-7".;

@BobobUnicorn
Copy link
Author

Personally I'm hesitant to add more complexity to TemplateStringsArray - especially since it's inevitable that people will overuse this to write slow DSL parsers on long template strings.

That said, it does feel like there should be a way to capture the contents of a template string today.

Well, in some sense, this PR is really intended to support "slow" DSL parsers; in particular, a library that I've been contributing to has wanted to support compile-time checking of template literals (and their parts) because of the particular API:

const TEMPLATE = html`
  <${MyComponent} prop:foo=${someValue} />
`;

and there's no compile-time validation that someValue actually satisfies the type of MyComponent#foo.

@sandersn
Copy link
Member

@DanielRosenwasser from the design meeting notes, it sounds like we want to hold this PR to get more discussion on the issue. Is that correct?

@BobobUnicorn
Copy link
Author

BobobUnicorn commented Aug 20, 2021

I took a look at the meeting notes #45504, and I did note this bit:

  • Favorite markup/query language examples =
    • "I want to write a full HTML parser and provide the same checking that JSX gives"
    • "I want to write a full GraphQL parser and provide exact API shapes back"
    • This is a moderate 👎 - we think we'd be encouraging people to use the wrong tools to solve these problems

I am curious about what the right tool is then? There doesn't really seem to be a great alternative to doing this. In particular, the problem is that of run-time equivalence; the library is intended to be usable both with plain JS and TS. JSX cannot be used without compiling, and furthermore the use-case of safe HTML templates was among the original design considerations that TC39 looked at when adding this feature to the spec. Tagged template literals are a very natural API for native JS libraries (see also: graphql-tag, html tag).

The approach taken by Svelte/Vue (custom language servers) or tsserver plugins doesn't really scale; writing transformers to turn syntax into JSX seems like a significant amount of work, especially for smaller projects. This approach also doesn't even allow for correctness checks at compile-time.

@freshollie
Copy link

Also a use case in the wild which currently suffers from this limitation: https://github.com/arcanis/graphql-typescript-integration and https://www.graphql-code-generator.com/docs/presets/gql-tag-operations

@nettybun
Copy link

nettybun commented Aug 29, 2021

I just stumbled into this while experimenting so thought I'd toss in a "use-case".

I'm creating value containers that have explicit names:

const name = val`userName`("");        // type Val<"userName",string>
const age = val`userAge`(100);         // type Val<"userAge",number>
const count = val``(0);

Tagging names like this is useful since containers will be passed around and prop-drilled - the variable name const xyz = is only defined in the local scope and not actually part of the item.

I know there's a million ways to do this like val("name", "...") or val("name")("...") etc but I naturally reached for a tag template version because I want to convey that it's the tag on the container and not the value - something like val("name"... could be read as if it's storing "name" as a value into the container...

I avoid variable names altogether like this:

const state = pack(
  val`name`("Willow"),
  val`age`(100),
  val`weekday`('Monday'),
)
// typeof state = { name: Val<"name", string>, age: Val<"age", number>, weekday: Val<"weekday", string> }

So I can just do console.log(state.name()); // "Willow". This means names aren't only a runtime thing. I use them at compile-time to check for naming collisions / duplicates when packing an object... I successfully wrote a pack type definition that errors if there's a duplicate name, but until this PR is merged it only works with the val("name")("...") format...

@sandersn
Copy link
Member

@DanielRosenwasser did we discuss this? If so, what was the outcome? I'm going through community PRs and trying to decide whether to keep it open.

@DanielRosenwasser
Copy link
Member

It was discussed at #45504. I think the outcome is that we're still not yet confident that we want to make this change, so for now I'm going to close this. We can revisit in the future if we ever change our minds. We do appreciate the PR though @BobobUnicorn.

PR Backlog automation moved this from Not started to Done Oct 20, 2021
@BobobUnicorn
Copy link
Author

thanks for letting me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

Type safety with TemplateStringsArray and tag functions
7 participants