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

Type containing templated, branded string is not assignable to itself #61098

Open
HansBrende opened this issue Feb 1, 2025 · 5 comments Β· May be fixed by #61113
Open

Type containing templated, branded string is not assignable to itself #61098

HansBrende opened this issue Feb 1, 2025 · 5 comments Β· May be fixed by #61113
Labels
Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases
Milestone

Comments

@HansBrende
Copy link

HansBrende commented Feb 1, 2025

πŸ”Ž Search Terms

brand branded tag tagged template string not assignable extends different type name unrelated

πŸ•— Version & Regression Information

  • This changed between versions 5.0.4 and 5.1.6

⏯ Playground Link

https://www.typescriptlang.org/play/?#code/FAUwHgDg9gTgLgAgDYkQQwFwIBQB4AqAfNgJQIC8hC+C4cIAdgCYDOCABgCQDeA5GrwQAyBNwSYEARgQBfGewQB+KQiwAmMuQQMArkiQBCANyhIsRCkQAjLHiKkKVGnUasOPfoJFiJ0uQuVpdU1tPUMTYDQKBCsjIA

πŸ’» Code

export let a: (<T>() => T extends `${'a' & { a: 1 }}` ? 1 : 2) = null!;
export let b: (<T>() => T extends `${'a' & { a: 1 }}` ? 1 : 2) = null!;

a = b; // ❌ Error!

πŸ™ Actual behavior

Typescript error:

Type
'<T>() => T extends `${"a" & { a: 1; }}` ? 1 : 2' is not assignable to type
'<T>() => T extends `${"a" & { a: 1; }}` ? 1 : 2'.
Two different types with this name exist, but they are unrelated.

πŸ™‚ Expected behavior

No error

Additional information about the issue

I would expect that, as a general principle, any type which is copy-and-pasted from one place to another should be assignable to itself. (With the exception of unique symbol, since by design it's supposed to not do what I just said.) This is perhaps the most lax notion of assignability that could possibly exist, I would think.

Note that, as soon as I remove either the outer template or the inner brand, assignability works as expected once again.

@HansBrende
Copy link
Author

Note: bisecting the issue reveals that the regression was introduced in #52836.

@HansBrende
Copy link
Author

Update: appears this issue is also present for the string mapping types like Uppercase, not just template strings. E.g.:

export let a: <T>() => T extends Uppercase<'a' & { a: 1 }> ? 1 : 2 = null!;
export let b: <T>() => T extends Uppercase<'a' & { a: 1 }> ? 1 : 2 = null!;

a = b; // ❌ Error!

@HansBrende
Copy link
Author

I believe this issue can be fixed by inserting the following code at line 23006 of checker.ts, in the function structuredTypeRelatedToWorker:

if (sourceFlags & TypeFlags.TemplateLiteral) {
    if (arrayIsEqualTo((source as TemplateLiteralType).texts, (target as TemplateLiteralType).texts)) {
        const sourceTypes = (source as TemplateLiteralType).types;
        const targetTypes = (target as TemplateLiteralType).types;
        result = Ternary.True;
        for (let i = 0; i < sourceTypes.length; i++) {
            if (!(result &= isRelatedTo(sourceTypes[i], targetTypes[i], RecursionFlags.Both, false))) {
                break;
            }
        }
        return result;
    }
}
if (sourceFlags & TypeFlags.StringMapping) {
    if ((source as StringMappingType).symbol === (target as StringMappingType).symbol) {
        return isRelatedTo((source as IndexType).type, (target as IndexType).type, RecursionFlags.Both, false);
    }
}

Let me know if you want me to open a PR.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases labels Feb 4, 2025
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Feb 4, 2025
@RyanCavanaugh
Copy link
Member

We can look at a PR, but we won't merge it if it re-regresses #52345

HansBrende added a commit to HansBrende/TypeScript that referenced this issue Feb 4, 2025
@HansBrende HansBrende linked a pull request Feb 4, 2025 that will close this issue
@HansBrende
Copy link
Author

@RyanCavanaugh cool, PR submitted! Not sure how to test if the fix re-regesses #52345 however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants