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

Restrict template literal interpolation expressions to strings #30239

Open
5 tasks done
jamietre opened this issue Mar 6, 2019 · 46 comments
Open
5 tasks done

Restrict template literal interpolation expressions to strings #30239

jamietre opened this issue Mar 6, 2019 · 46 comments
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@jamietre
Copy link

jamietre commented Mar 6, 2019

Search Terms

"template literal"

There are many hits, some which seem related, but everything I could find was a much bigger ask or wider in scope

Suggestion

Add a compiler option to enforce using only strings in ES6 string template literals

Use Cases

When using string literals, any variables are coerced to strings, which can lead to undesirable behavior.

As far as I can tell there's no way to avoid this behaviour. In the spirit of tying everything, I'd prefer that only actual string types are permissible for use in string templates to avoid accidental coercion by passing null, undefined or object types that may have unexpected string representations, and force users to explicitly convert them to strings.

Examples

For example:

function formatName(name: string | null): string {
   return `Name is: ${name}`;
} 

formatName(null)  === "Name is: null"

Ideally the compiler would fail since name can be null.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@yortus
Copy link
Contributor

yortus commented Mar 7, 2019

So `Step ${i} of ${count}` would have to be written as `Step ${i.toString()} of ${count.toString()}`?

I actually think this is a context where coercion to string is clearly expected, and it makes template literals easier to read and work with.

If you want the build to fail if name is null, then why in your example is name explicitly nullable? Perhaps the example could be clearer to show a compelling situation.

If its just about output format, you could write something like return `Name is: ${name || 'not supplied'}`;.

@jamietre
Copy link
Author

jamietre commented Mar 7, 2019

This is a contrived example; obviously I wouldn't write a function like this. In real work, we deal with complex data structures that have many fields of differing types. We process data that is far removed from its type definitions. That's the whole point of static typing.

I wouldn't write a function so narrowly purposed that allowed null, of course, but I might have a React component that takes a lot of props, some of which can be null or are optional, but I will still eventually use string templates to format data involving those fields. Unless I wrapped every single usage of a string template literal in a function that took a string arg for every field, I would face this risk.

The point of type safety is to protect you from mistakes. If people always correctly grokked the type of every variable they used in practice, then we wouldn't need typescript at all. But people don't. I might have a structure:

type Person = {
   firstName: string;
   lastName: string;
   middleName?: string; 
   address: Address;
}

If I did this:

const info = `${data.firstName} ${data.middleName} ${data.lastName} lives at ${data.address}`

I get

"Joe undefined Blow lives at [Object object]"

This would be a very easy mistake to make.

Personally, there's never a time when I want type coercion. I don't see why this is a place you'd want it, any more than you'd want it when assigning anything else to a variable of type string. String literal templates only deal with strings; it seems well within the purpose of TypeScript to enforce that we can only pass them strings.

BTW, the use case that this came up had to do with refactoring. I had to take an existing type and change it from string to string | null. If string template literal types were typesafe, the compiler would immediately fail in every place it was used in a string literal, as it currently would fail everywhere else it was no longer safe to use. Without the ability to type string template literals, you can't ever safely refactor code involving them when widening types.

@jamietre
Copy link
Author

jamietre commented Mar 7, 2019

Regarding this:

So `Step ${i} of ${count}` would have to be written as `Step ${i.toString()} of ${count.toString()}`?

I'd be fine with this - thought I'd probably say Step ${String(i)} of ${String(count)}

But I think it would also be reasonable to accept both number and string types, and perhaps boolean, since their string equivalent is reasonably well defined and generally purposeful.

But I would never want null, undefined, or object to be allowed without my explicitly coercing it. I can't think of any situations where I'd intentionally want to convert those types to a string through coercion by default.

I thinks this should be optional, as a compiler directive, like many other options that enforce stricter typing requirements.

@yortus
Copy link
Contributor

yortus commented Mar 7, 2019

I agree that implicit coercion of null, undefined, and object types to string is almost always an error in the code. But even then there are exceptions - such as when the object defines its own toString method, or when the template literal is tagged.

@jamietre
Copy link
Author

jamietre commented Mar 7, 2019

I also agree that there can be situations where you intentionally want to do that. But in practice, mistakes happen far more than those use cases.

I'd much rather say address.toString() if I really wanted that, then have no type checking for any string templates, just like I would have to anywhere else in my code that I wanted to convert address to a string.

Refactoring is really where it becomes a killer. When I went through the exercise today of having to manually inspect every bit of code that consumed the structure I'd refactored to see if it was involved in any templates, and also wondering if there were any derived usages I missed, it felt like a big hole.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Mar 7, 2019
@RyanCavanaugh
Copy link
Member

The core problem here is that people add custom toString methods with "good" output with reasonable frequency. The "real" solution would be a way to mark either blessed or cursed toString methods so that e.g. we can detect that Object#toString is a bad call but MyCustomPointWithNiceToString#toString is a good call.

@jamietre
Copy link
Author

jamietre commented Mar 7, 2019

The usage of good toString() seems like it is in conflict with general good practice in typescript. You can't do this:

let x: string = '';
x = someObjectThatHasToString;

You'd have to say:

x = someObjectThatHasToString.toString()

I don't see why you'd want to treat string templates any differently as part of a comprehensive static typing system. This is the only place you're allowed to not explicitly call toString() in order to get the output of that method in typescript.

I don't really like "toString()" anyway as part of production code - while it's handy for console logging or debugging interactively, I wouldn't want to use it for real output. Overriding the prototype method provides engineers with no information about what's actually being output for the object. Anything that's not trivial almost certainly has more than one possible format in which it's output could appear, so I think it's much better to use expressive names for string formatting methods.

Again - I'd much rather be required to explicitly invoke a method when using an object in a string template. In my perfect TypeScript world there's no "good" toString

I certainly understand some people might feel differently, hence making it optional seems reasonable for this reason and backward compatibility. But at the same time I think the notion of being able to disable coercion for string templates has value and would benefit users who prefer better type safety over convenience. I can't think of any other exceptions to "no coercion" (other than using any) and as such it presents a risk that seems like it shouldn't be necessary when using string templates and refactoring code.

@jamietre
Copy link
Author

Here's something else interesting related to this. In designing classes, we'd actually like to forbid the use of string coercion. It seems like this should be possible:

class Foo {
  private value: string;
  constructor(value: string) {
    this.value = value;
  }
  toString(): never {
    throw new Error();
  }
  toNamedFormat(): string {
    return this.value;
  }
}

const foo = new Foo('abc');
const x = `foo is ${foo}`;

But TypeScript has no problem with this. Probably I just don't understand the totality of how never works. Is there any way to create a method with a return type that would be forbidden by string templates?

@samtstern
Copy link

I ran into this with something like:

console.log(`Name is ${name}`);

Name was not defined in my scope, but in lib.dom.d.ts it was defined as:

declare const name: never;

So my code compiled but at runtime I got:

ReferenceError: name is not defined

Result: firebase/firebase-tools#1241

@maexrakete
Copy link

maexrakete commented Jun 14, 2019

We had a similiar issue. We have split shared code into libs, which are used in several services. Every lib is used in at least 2 services. Someone expanded an interface, replacing a string with an object.

In our code we used a string template like this Foo ${bar}, where bar changed to an object. So what I expected was, that the compiler throws an error. Instead we had an object Object in production. I think, if you have a custom toString method, it's okay to call it in this context instead of intransparently breaking code.

@RazgrizHsu
Copy link

on node:

const moment = require( 'moment' );
console.info( `show: [${ moment.utc() }]` )

//output: 'show: [Wed Oct 30 2019 11:30:00 GMT+0000]'

on ts-node:

const moment = require( 'moment' );
console.info( `show: [${ moment.utc() }]` )

//output: 'show: [1572435290801]'

But If you use "target": "es2015" then you will have native template literals

how to change this behavior on other target?

@DanielRosenwasser DanielRosenwasser changed the title Type checking of ES6 string template literals Restrict template literal interpolation expressions to strings Nov 1, 2019
@zachlysobey
Copy link

I've also run into some frustration with this. Not only with template strings, but similarly with the + operator. 'hi ' + {} does not complain.

One possible solution would be to add a way to disable implicit coercion on individual types defined in TS (as opposed to native ones)? Just a thought.

@samhh
Copy link

samhh commented Nov 19, 2019

Just hit this issue myself. It's very easy to start relying on the compiler to catch silly stuff, and miss something inane like forgetting to call a function; it even made it through review unnoticed.

In my case, our CI acceptance tests were failing with a very peculiar looking output... turns out the template literal was coercing a function, per the above.

If you've got the following:

const fn = (): string => 'a string';
const x = `/blah/blah/${fn()}`;

And you forget to call fn, instead interpolating ${fn}, the compiler really should flag it up. I'd posit it's far more likely to be accidental than not, and if someone really wants to toString their function/object for some reason - surely a very rare use case? - then they can always do so manually.

In terms of safety, flexibility, and being as helpful as possible to as many developers as possible, I think this warrants a fix.

@stevemao
Copy link

Is there any workaround for this now?

@wongjiahau
Copy link

I need this too

@ark120202
Copy link

typescript-eslint has a restrict-template-expressions rule to catch this

@zamb3zi
Copy link

zamb3zi commented Mar 10, 2020

This is also a problem for us where we have a lot of methods and getters for building template literals. If the arguments are omitted we get runtime errors. Following is my naive attempt:

const neverString = <T>(arg: T) => arg as T & { toString: never };

const f = neverString((x: number) => `${2 * x}`);

`${f(5)}`; // OK as expected
`${f.toString()}`; // Error as expected
`${String(f)}`; // OK - should be error?
`${f}`; // OK - should be error?

I'm aware that there's a countervailing expectation that String(...) should always be allowed however.

@SimonAlling
Copy link

SimonAlling commented Sep 30, 2020

This seems to me like one of the weakest links in TypeScript's type system – in the weak–strong sense related to the presence and semantics of implicit type coercion (not static–dynamic, explicit–inferred, expressive–inexpressive or any other type-system-defining dimensions that are often confused with each other or used ambiguously).

What often happens for me in practice is this:

text.ts:

export default {
    url: `https://example.com`,
} as const;

main.ts:

import T from "./text";

console.log(`Your URL: ${T.url}`);

Then I refactor/add a new feature/whatever such that text.ts looks like this instead:

export default {
    url: (user: string) => "https://example.com/" + user,
} as const;

Or perhaps this:

export default {
    url: {
        external: "https://foo.bar",
        profile: (user: string) => "https://example.com/" + user,
    },
} as const;

main.ts now prints something like Your URL: user => "https://example.com/" + user or Your URL: [object Object] – and the type checker doesn't help me to detect this at all, even though it could have. Especially when looking at this from a Haskell perspective, I really expect a type error, as I'm used to being protected against implicit and/or nonsensical stringification in that language:

$ ghci
> "" ++ id

<interactive>:1:7: error:
    • Couldn't match expected type ‘[Char]’ with actual type ‘a0 -> a0’
    • Probable cause: ‘id’ is applied to too few arguments
      In the second argument of ‘(++)’, namely ‘id’
      In the expression: "" ++ id
      In an equation for ‘it’: it = "" ++ id

> "" ++ show id

<interactive>:2:7: error:
    • No instance for (Show (a0 -> a0)) arising from a use of ‘show’
        (maybe you haven't applied a function to enough arguments?)
    • In the second argument of ‘(++)’, namely ‘show id’
      In the expression: "" ++ show id
      In an equation for ‘it’: it = "" ++ show id

(GHC even points out the actual cause of the problem in these cases, namely that I haven't applied id.)

@osdiab
Copy link

osdiab commented Mar 11, 2021

I would extend this suggestion to also include string concatenation with the + operator.

That said in addition to the rule @ark120202 mentioned, this ESLint rule that specifically handles objects without a meaningful toString() method in both template literal and + operator cases exists:

https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-base-to-string.md

@mindplay-dk
Copy link

I'd like to add a much worse example of this:

let a = ["one", "two"];

let s = `<ul>` + a.map(v => `<li>${v}</li>`) + `</ul>`;

It's not immediately obvious that there's anything wrong with this code, and TS doesn't think so either.

The rendered result, however, includes commas between the <li> elements, because I forgot to .join("") the array.

I was quite surprised that this is allowed in the first place? While arrays and objects are capable of coercion via toString(), an array or object is not a string, and these coercions rarely result in anything meaningful. (notable exception may be number)

I would propose adding a new compiler option to prevent implicit object to string conversions - and consider making it default for strict mode.

I think there's an inspection for this in ESLint? I don't use ESLint and haven't felt the need - since I've decided to use TS for type-hecking, it would be nice if I didn't need additional tooling for something that is essentially just a type-check.

@Peeja
Copy link
Contributor

Peeja commented Aug 16, 2021

@mindplay-dk

I think there's an inspection for this in ESLint?

There is! In fact, there are two that can cover it: @typescript-eslint/restrict-template-expressions is the most directly applicable rule, but @typescript-eslint/no-base-to-string also covers it. It would be nice to have this built into TS itself, but doing it in ESLint works quite well, too.

I've decided to use TS for type-hecking

Mood.

@mindplay-dk
Copy link

Heck, I might even use it for type-checking.

@zenhob
Copy link

zenhob commented Aug 23, 2021

I just fixed an annoying bug that would have been caught by this. I was doing string interpolation to construct an URL in CSS. I can't imagine anyone ever intends to load url(undefined). Thanks @Peeja for the eslint rule links.

@andrejewski
Copy link

If you want a workaround here using TypeScript, here's a function:

function s (strings: Readonly<string[]>, ...values: Readonly<string[]>): string {
  let str = ''
  let i = 0
  for (; i < values.length; i++) {
    str += strings[i]
    str += values[i]
  }

  return str + strings[i]
}

s`${undefined}` // => Argument of type 'undefined' is not assignable to parameter of type 'string'.

I agree this behavior should be some sort of tunable strict option for the compiler. You can special case number but you know what I don't want to display to end-users? 1.7976931348623157e+308 or NaN. And toString on objects is never what I want.

@RyanCavanaugh RyanCavanaugh mentioned this issue Apr 4, 2022
5 tasks
@beauxq
Copy link

beauxq commented Apr 4, 2022

I would not want to limit it to strings.
I would just want to ban undefined and any object that has not overridden the default .toString()
If an object has overridden the default toString(), then it's fine for template literals, and concatenation with strings.

for objects:
shouldBeAllowed = objectInQuestion.toString !== {}.toString

@beauxq
Copy link

beauxq commented Apr 4, 2022

since there was a shortage of good motivating examples at the beginning of this thread:

async function get_f() {
    return "f";
}

(async function main() {
    const f = get_f();  // oops, forgot the `await`

    const oo = "oo";

    const foo = f + oo;  // or `${f}${oo}`

    console.log(foo);  // [object Promise]oo
})();

It's obvious that it's a type error where someone just forgot to await the promise.

@joehillen
Copy link

@beauxq How does that need a template?

const f = get_f();  // oops, forgot the `await`

It's obvious that it's a type error where someone just forgot to await the promise.

That's what a linter is for.

@beauxq
Copy link

beauxq commented Apr 4, 2022

@beauxq How does that need a template?

Did I suggest that anything needs a template? I don't understand this question.

That's what a linter is for.

It's a type error. That's what a static type analyzer is for. This is the first thing on the list of TypeScript design goals.

@samhh
Copy link

samhh commented Apr 5, 2022

It's obvious that it's a type error where someone just forgot to await the promise.

It's perfectly valid and reasonable to use promises without async/await sugar. The problem is coercion.

@beauxq
Copy link

beauxq commented Apr 5, 2022

It's perfectly valid and reasonable to use promises without async/await sugar. The problem is coercion.

That's correct. In this example, it's obvious that it's a type error where someone just forgot to await the promise. And the coercion is the reason the type error isn't found.

@samhh
Copy link

samhh commented Apr 6, 2022

In this example, it's obvious that it's a type error where someone just forgot to await the promise.

No, because the Promise value could have been (intended to be) used somewhere else. It's too presumptive and prescriptive to pin the problem on the lack of await.

@beauxq
Copy link

beauxq commented Apr 6, 2022

No, because the Promise value could have been (intended to be) used somewhere else. It's too presumptive and prescriptive to pin the problem on the lack of await.

No one is "pinning the problem on the lack of await". You're not reading the sentence that I've already posted twice so that you can read it more carefully. I have always been completely aware that a promise can be intended to be used without await.

@jamietre
Copy link
Author

jamietre commented Apr 6, 2022

When I created this issue I tried to show a common cases involving coercion, but out of context it's easy for them to be dismissed as contrived. I think the promise one is good, but maybe slightly confusing also because of the lack of context?

There are really two separate issues, though, that I don't think I thought through when I initially made the issue and probably should be considered separately. One is coercion of non-string values such as null and undefined, and possibly numbers and booleans, in a string-only context. It think there's room for debate about whether these should be automatically coerced. My preference remains that "null" and "undefined" at least should not be; I'd prefer having to wrap something in String() then not being able to write code that's guaranteed to only handle strings.

The other is the automatic use of native toString() method. This is the case with Promise objects, as well as anything that inherits Object.prototype. Promises have a toString() method that returns "[object Promise]". POJOs return "[object Object]". This is almost never what you want, and I don't think a type checker should ever allow automatic use of the native toString() method in a string context.

There are good linting solutions to this problem now, so honestly I don't care as much as I did 3 years ago, though I do still think it's within the goals of Typecript to forbid this usage, and I also think it's more consistent with typechecking in other contexts.

-- Type checking is enforced when assigning something to an existing string variable.
-- toString() is automatically used when concatenating strings, or within string templates

These are two different issues from an actual javascript standpoint (since the former would be changing the type of the variable); but from a "how do statically typed languages work" standpoint it seems inconsistent.

Examples:

class Thing {
 count(): number { ... }
}

const thing = new Thing();
// not what you wanted
console.log(`The count is: ${thing.count}`)
> "count() { return 2 }" 

// correct
console.log(`The count is: ${thing.count()}`)

let bar: string=""
bar = thing.count;
// Type '() => number' is not assignable to type 'string'.ts(2322)

bar = "" + thing.count;
// no problem here!

@SimonAlling
Copy link

I'd just like to mention that code examples become more readable with syntax highlighting. 🙂 It's easy to achieve in Markdown:

```ts
console.log("Hello, World!")
```

Result:

console.log("Hello, World!")

@kalda341
Copy link

This is a common source of bugs for us - and Typescript seems like the right place to be preventing them.
I really hope that the Typescript team implement this and #42983 with a compiler flag to enable it.

@wongjiahau
Copy link

I found a temporary workaround with Typescript v4.0.3 using Tagged templates.

export function s(strings: TemplateStringsArray, ...values: string[]): string {
  return strings.reduce(
    (result, string, index) => result + string + (values[index] ?? ""),
    ""
  );
}
let myString = s`hello world ${123} yo ${"lo"}`;
console.log(myString)

The above code will yield the following error:

temp.ts:7:32 - error TS2345: Argument of type 'number' is not assignable to parameter of type 'string'.

7 let myString = s`hello world ${123} yo ${"lo"}`;
                                 ~~~

This works because the type of ...values is restricted to string[].

The downside however is of course you have to import s and tag it with every template literal in the codebase.

Nonetheless, that can be automated using the Typescript parser found in Typescript ES Tree.

I'm not going to give every detail about the automation as it can be too lengthy. Let me know if you're interested.

jared-hughes added a commit to jared-hughes/DesModder that referenced this issue Jun 16, 2022
The issue was an object in a template string. It might be worth
requiring template strings to pass in *only* strings (or numbers?)
but this requires a bit of a workaround currently:

microsoft/TypeScript#30239 (comment)
@houchaowei
Copy link

Why even bother with this rule?

@houchaowei
Copy link

Why even bother with this rule?为什么还要打扰这个规则?

@samhh
Copy link

samhh commented Jul 25, 2023

Why even bother with this rule?

A string variable is interpolated. You refactor and it's now something else. You almost certainly want to change how it's being interpolated, but the compiler doesn't flag it.

So, the same as why (among other reasons) you want static type safety everywhere else.

@houchaowei
Copy link

Why even bother with this rule?

A string variable is interpolated. You refactor and it's now something else. You almost certainly want to change how it's being interpolated, but the compiler doesn't flag it.

So, the same as why (among other reasons) you want static type safety everywhere else.

What I understand is that I'm refactoring the variable, but when I use a template string, it should be forced to be converted to string now. Can you give me an example in response to what you said?

@jamietre
Copy link
Author

jamietre commented Jul 25, 2023

What I understand is that I'm refactoring the variable, but when I use a template string, it should be forced to be converted to string now. Can you give me an example in response to what you said?

This thread already includes many examples, have you read it from the beginning? One of the central purposes of TypeScript, at a very high level, is that we want to prevent "forcing" conversion of one type to another and allow engineers to require such conversions to be explicit.

@houchaowei
Copy link

What I understand is that I'm refactoring the variable, but when I use a template string, it should be forced to be converted to string now. Can you give me an example in response to what you said?

This thread already includes many examples, have you read it from the beginning? One of the central purposes of TypeScript, at a very high level, is that we want to prevent "forcing" conversion of one type to another and allow engineers to require such conversions to be explicit.

Have read it from the beginning, but not very carefully. I ran into a problem as follows:
types/index.ts

export type Params = Partial<{
  questionnaireId: string
}>

components/chat-page/test.tsx

import {FC} from 'react'

import {Params} from '@/types'

interface IProps {
  params: Params
}
const Test: FC<IProps> = props => {
  const {params} = props
  if (!params) return null
  if (!params.questionnaireId) return null
  const url = `xxx${params.questionnaireId}}`
  return <>{url}</>
}

export default Test

The error is reported as shown:
018aa66d-2b49-4bbb-9fd6-38b78e6e7605

In this case, the string has already been inferred, so why is there an error?

@jamietre
Copy link
Author

Have read it from the beginning, but not very carefully. I ran into a problem as follows:

Please read it carefully if you want to understand this proposal.

In this case, the string has already been inferred, so why is there an error?

It's really not possible to know why the lint rule isn't working as expected without further context including your eslint and tsconfig files, typescript version, and so on. This exact same example works fine for me trivially.

More generally, though, this is not a support forum. You might try asking this question on stack overflow.

@houchaowei
Copy link

Have read it from the beginning, but not very carefully. I ran into a problem as follows:

Please read it carefully if you want to understand this proposal.

In this case, the string has already been inferred, so why is there an error?

It's really not possible to know why the lint rule isn't working as expected without further context including your eslint and tsconfig files, typescript version, and so on. This exact same example works fine for me trivially.

More generally, though, this is not a support forum. You might try asking this question on stack overflow.

You're right. Thank you.

@mkantor
Copy link
Contributor

mkantor commented Oct 3, 2023

In template literal types, interpolations are required to be string | number | bigint | boolean | null | undefined:

type A = `before${unknown}after` // error
type B = `before${string | number | bigint | boolean | null | undefined}after` // ok

If a compiler option to require using only strings in template literal interpolations feels too restrictive, perhaps the option could instead require string | number | bigint | boolean | null | undefined, which should catch most mistakes and would nicely align the rules for type-level and value-level literals.

@goldingdamien
Copy link

Some URLs here are broken.
The related typescript-eslint solution URLs are below:

https://typescript-eslint.io/rules/restrict-template-expressions/
https://typescript-eslint.io/rules/no-base-to-string/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests