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

Incorrect behavior of template strings when compiling to pre-ES6 targets #43686

Closed
ghost opened this issue Apr 15, 2021 · 5 comments
Closed

Incorrect behavior of template strings when compiling to pre-ES6 targets #43686

ghost opened this issue Apr 15, 2021 · 5 comments
Assignees
Labels
Bug A bug in TypeScript

Comments

@ghost
Copy link

ghost commented Apr 15, 2021

Bug Report

πŸ”Ž Search Terms

template string literal interpolation

πŸ•— Version & Regression Information

I tested this behavior locally on v4.2.4 and in the playground on Nightly.

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

const x = {
    valueOf: () => "valueOf()",
    toString: () => "toString()"
};

// ES6
`${x}` === "toString()"; // true
// compiles to native ES6 template string, which uses toString() internally

// ES5 and lower
`${x}` === "valueOf()"; // true
// compiles to a series of + concatenations, e.g. `foo ${x} bar` == "foo " + x + " bar"

πŸ™ Actual behavior

When using template strings, if the compilation target does not natively support them (i.e. pre-ES6), the template string will be compiled into a series of concatenations using the + operator, which internally uses the valueOf() method to convert to a string.

The ECMAScript standard for template strings specifies that

The string conversion semantics applied to the Expression value are like String.prototype.concat rather than the + operator.

String.prototype.concat uses toString() internally, as can be seen in the playground code.

If valueOf() and toString() methods return different values, the behavior will be different between different targets, leading to breakages and bugs.

πŸ™‚ Expected behavior

On targets that require polyfilling of template strings, compile them into a series of concat() operations to ensure consistent behavior across targets and compliance with the ECMAScript standard.

@andrewbranch andrewbranch added the Bug A bug in TypeScript label Apr 15, 2021
@andrewbranch andrewbranch added this to the TypeScript 4.4.0 milestone Apr 15, 2021
@DanielRosenwasser
Copy link
Member

Is this a duplicate of #27460? If you use modules (include an import or export in your file), then you should get the expected behavior.

@ghost
Copy link
Author

ghost commented Apr 23, 2021

Hi Daniel,

I do not believe this is a duplicate of #27460 or has anything to do with caching. As far as I could investigate, the issue here is that depending on the compilation target, different methods of an object will be called when interpolated in a template string due to incorrect polyfilling. I came across this issue in a module in my production code to begin with. πŸ˜‰

For some context, I'm currently using the discord.js library, which has a class called GuildMember. If you call toString() on an instance of GuildMember, it gets converted into a mention tag that can be sent in a Discord message. It also inherits a valueOf() method from a base class, which just returns the member's id. The documentation and StackOverflow show that the intended way to mention people is to interpolate GuildMember objects in template strings. However, due to the issue at hand, I was getting user ids instead of mention tags in my messages (I had my compilation target set to ES5). Needless to say, that was pretty confusing.

The following is a complete module which triggered the issue. There is a bunch of unrelated code there, with the offending line at the bottom.

import { CommandRequest } from "../request";
import { Permissions } from "discord.js";
import { CommandParameter, Command } from "../command";
import MentionConverter from "../type_converters/MentionConverter";
import StringConverter from "../type_converters/StringConverter";

export default <Command>{
    parameters: [
        new CommandParameter("target user id", MentionConverter),
        new CommandParameter("role name", StringConverter),
        new CommandParameter("role color", StringConverter),
    ],
    permissions: [Permissions.FLAGS.MANAGE_ROLES],

    async execute(
        { source }: CommandRequest,
        target_user_id: string,
        role_name: string,
        role_color: string
    ): Promise<void> {
        const target_member = source.guild?.members.cache.get(target_user_id);
        if (!target_member) return;

        const role_options = {
            data: {
                name: role_name,
                color: role_color,
                permissions: 0,
                mentionable: true,
            },
        };
        const role = await source.guild?.roles.create(role_options);
        if (!role) return;

        target_member.roles.add(role.id).then(() => {
            source.channel.send(
                // target_member interpolated as `<@!some_id>` in ES6, `some_id` in ES5
                `Gave ${target_member} a new role \`${role.name}\`` 
            );
        });
    },
};

@DanielRosenwasser
Copy link
Member

Whoops, sorry, I misread the issue.

@DanielRosenwasser
Copy link
Member

It does seem to be the same as #39744 though.

@ghost
Copy link
Author

ghost commented Apr 24, 2021

Ah, that would be it, yes. It didn't come up when I searched for prior reports before submitting, must've been a poor choice of keywords on my part. Sorry about that. Closing as duplicate.

@ghost ghost closed this as completed Apr 24, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants