Skip to content

Add more context to unique symbols that appear in type errors #61099

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

Open
6 tasks done
dberlin opened this issue Feb 2, 2025 · 6 comments
Open
6 tasks done

Add more context to unique symbols that appear in type errors #61099

dberlin opened this issue Feb 2, 2025 · 6 comments
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

@dberlin
Copy link

dberlin commented Feb 2, 2025

πŸ” Search Terms

unique symbol type error
I went through every bug i could find to see if someone suggested anything like this - no luck.

βœ… Viability Checklist

⭐ Suggestion

Aa part of tracking down sveltejs/svelte#15182 and sveltejs/svelte#13760, you can see the error messages from the typescript compiler around conflicting unique symbols correctly state that the types are different. But they give you no real context about where the unique symbols are from, or even give the unique symbols different numbers.

So you get something like (ignore the annoying string that svelte has added to the type):

Image

And if you are a normal developer, you are just like "uh, okay" - the types look identical because by name, they are! (Both the different unique symbols are named SnippetReturn).

The actual part that matters is really that part that states that the unique symbol is not assignable to unique symbol. At the very bottom.

But the really important part, that you can't tell here, is that the unique symbol it's talking about in "& unique symbol" and the one in 'type "unique symbol"' are different unique symbols.

This was hard enough to tell that nobody actually understood what this means., and just assumed it was breakage elsehwere.

All of this is coming from the typescript compiler/parser, and you can see this with the repro in 15182 that is just a bare ts file. Running tsc on the project will give you a similar message. tsc happens to name the overall containing types at the top, which is nice. But this only looks nice because of the import paths. In the real errors, you either don't get the import statements, or you see Type 'import("svelte").Snippet<[]>' is not assignable to type 'import("svelte").Snippet<[]> at the top which helps make you think the tooling is broken, not the code.

It would be really nice if, for unique symbols (which are all unique), it gave you more context.

Like Type '{ '{@render ...} must be called with a Snippet': "import type { Snippet } from 'svelte'"; } & (unique symbol declared at line 57 of file bar.ts)' is not assignable to type '(unique symbol declared at like 48 of file foo.ts)'

So it's totally obvious they are not the same unique symbol.
Maybe even add "Different instances of unique symbols are never compatible" at the end to help.

If this is too hard, even just something like:
Type '{ '{@render ...} must be called with a Snippet': "import type { Snippet } from 'svelte'"; } & (unique symbol #1)' is not assignable to type '(unique symbol #2)'. #1 and #2 are two different instances of unique symbols, and different instances of unique symbols are never compatible

So that it was similarly obvious, though not quite as helpful.

Another option would be to point out the SnippetReturn's it's talking about are not the same (SnippetReturn is the thing that is typeof <unique symbol>)

Type '{ '{@render ...} must be called with a Snippet': "import type { Snippet } from 'svelte'"; } & typeof SnippetReturn (SnippetReturn is an instance of a unique symbol) ' is not assignable to type '{ '{@render ...} must be called with a Snippet': "import type { Snippet } from 'svelte'"; } & typeof SnippetReturn (SnippetReturn is a *different* instance of a unique symbol)'. Two different types with this name exist, but they are unrelated.

This is even less helpful, but still IMHO an improvement.

πŸ“ƒ Motivating Example

Tracking down a svelte bug - sveltejs/svelte#15182
This bug went unnoticed despite a number of reports in part because the error message didn't clearly help understand what was broken, and so folks wrote it off. it was worsened by the fact that the typeof's of the different unique symbols had the same name, making the error message look like a bug or weird failure.

Had it contained the filenames and declarations, someone would probably have looked and it would have been instantly obvious that it not was random weirdness, but a real bug.

πŸ’» Use Cases

  1. What do you want to use this for?

Better error messages

  1. What shortcomings exist with current approaches?

The current error message is missing enough that there are concrete cases of very smart developers missing the real bug.

  1. What workarounds are you using in the meantime?

Using people like me with lots of compiler experience who are willing to jump very far down crazy compiler and language server rabbit holes when bugs look weird.

@RyanCavanaugh
Copy link
Member

I'm not sure how to reduce that to an isolated repro; can you provide something that doesn't require all of Svelte's types to repro? Doing something like this

/a.ts
export const x: unique symbol = Symbol();
export const y: unique symbol = Symbol();
import { x, y } from "./a.js";

const m: typeof x = y;

gives me the legible error

Type 'typeof import("D:/Throwaway/uniq/a").y' is not assignable to type 'typeof import("D:/Throwaway/uniq/a").x'

@dberlin
Copy link
Author

dberlin commented Feb 5, 2025

So it's really hard to reduce it and get it to lose the import path printing in the way you see above, or get it to say the import paths are the same.

However, if we ignore that portion for a second, here's a very small version to show the rest of the error message (IE not distinguishing between the unique symbols themselves), just to give you a small repro of that.

Here's how it compares to what is going on in svelte:

The svelte/types and svelte/ parts are exactly how it works in svelte (ie that's where the two types are coming from, and the structure is the same)
The paths in tsconfig.json, import paths, and way it happens in foo.ts and bar.ts is not how it happens in svelte, and arguably quite stupid.
Because svelte is a compiler, and you asked to reduce it without all of svelte involved, i'm doing the best i can to get there :)

Compared to the real error message. the main different is the import path printing:
This is either coming from tsconfig options (messing around with paths, baseurl, etc), the mixture of import types (bare, module, etc) that exist in svelte, or server related weirdness (svelte does not use TSC, it has a language server that is simiilar to tsserver that it is using), or some combination thereof.

That part is hard to track down, i'm working on it.

error:

bun run tsc -p ./tsconfig.json
bar.ts:3:5 - error TS2345: Argument of type 'import("svelte").Snippet<[string, string]>' is not assignable to parameter of type 'import("/Users/dannyb/sources/svelte-broken/svelte/index").Snippet<[string, string]>'.
  Type '{ "{@render ...} must be called with a Snippet": "import type { Snippet } from 'svelte'"; } & typeof import("svelte").SnippetReturn' is not assignable to type '{ "{@render ...} must be called with a Snippet": "import type { Snippet } from 'svelte'"; } & typeof import("/Users/dannyb/sources/svelte-broken/svelte/index").SnippetReturn'.
    Type '{ "{@render ...} must be called with a Snippet": "import type { Snippet } from 'svelte'"; } & unique symbol' is not assignable to type 'unique symbol'.

tsconfig.json:

{
  "compilerOptions": {
    "paths": {
      "svelte": ["./svelte/types"]
    }
  }
}

svelte/types/index.d.ts

declare module "svelte" {
  const SnippetReturn: unique symbol;
  export interface Snippet<Parameters extends unknown[] = []> {
    (
      this: void,
      ...args: number extends Parameters["length"] ? never : Parameters
    ): {
      "{@render ...} must be called with a Snippet": "import type { Snippet } from 'svelte'";
    } & typeof SnippetReturn;
  }
  export function createSnippet(): Snippet<[string, string]>;
}

svelte/index.d.ts

export declare const SnippetReturn: unique symbol;
export interface Snippet<Parameters extends unknown[] = []> {
  (
    this: void,
    ...args: number extends Parameters["length"] ? never : Parameters
  ): {
    "{@render ...} must be called with a Snippet": "import type { Snippet } from 'svelte'";
  } & typeof SnippetReturn;
}

foo.ts

import type { Snippet } from "./svelte/index.d.ts";

export function foo(param: Snippet<[string, string]>) {}

bar.ts

import { createSnippet } from "svelte";
import { foo } from "./foo";
foo(createSnippet());

@RyanCavanaugh
Copy link
Member

// in svelte/types/index.d.ts
declare module "svelte" {
  const SnippetReturn: unique symbol;

// in svelte/index.d.ts
export declare const SnippetReturn: unique symbol;

There are two unique symbol declarations that are both the result of importing SnippetReturn from "svelte" ? Am I reading this right?

@dberlin
Copy link
Author

dberlin commented Feb 5, 2025

Yes. They (svelte) have created two unique symbols with the same name. They then create two Snippet types that return different types as a result of being based on different unique symbols. So the two types you see as textually identical in the original (and minimized) error message are from two different files, really are different types. They just look identical (once the import statements look the same or go away) because nothing printed out distinguishes the two unique symbols.
That is what led to my suggestion - once you make the imports look the same, it's the only thing that can be usefully distinguished, at least in this error message.

It is, of course, only a suggestion, and you can say "this is such an edge case it's not worth caring about because who in their right mind does this" or whatever ;)

I have, of course, already reported the bug to the svelte folks and strongly suggested to them that doing this is broken, and that they should instead create a single unique symbol, and share the typeof between the two places. Or share the snippet type. Or anything other than this :)

@dberlin
Copy link
Author

dberlin commented Feb 7, 2025

So figured out where this error message would go.
It does not look that tricky, just special casing the "two unique symbols are incompatible" case in the part of the checker issuing the errors about type1 incompatible with type2.
Unless folks think this is a truly horrible idea, i'll implement it, send a PR, and if folks decide it's not worth it, that's fine. It doesn't seem like a lot of work, so if it gets thrown away, i'm good with it :)

@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 7, 2025
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Feb 7, 2025
@RyanCavanaugh
Copy link
Member

Yeah, no objections to that. Even if the scenario is one that ideally shouldn't happen in the first place, the error experience is sort of broken beyond self-rescue here.

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

No branches or pull requests

2 participants