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

svelte accidentally contains two incompatible Snippet types that cause type checking failures #15182

Open
dberlin opened this issue Feb 2, 2025 · 26 comments
Labels
awaiting submitter needs a reproduction, or clarification

Comments

@dberlin
Copy link

dberlin commented Feb 2, 2025

Describe the bug

The use of unique symbol as the type of SnippetReturn means that every instance refers to a unique symbol. They are not structurally compatible with each other (unlike basically everything else in typescript)

As such, you have to use typeof on them to get a reference to the same unique symbol.

Which svelte does. Or tries to.
Unfortunately, however, it contains two different instances of the SnippetReturn symbol - one in src/index.d.ts, and one in types/index.d.ts.

Both contain const SnippetReturn: unique symbol (of some form), which means both create different unique symbols
In turn, the Snippet types that exist in each file now have different return values, even though they are typeof'ing that symbol, and because of how unique symbols work, those return values are incompatible with each other.

This causes issues that pop up occasionally on the issue tracker such as #13670.

The above is the cause of this issue (and a few others).
The solution, AFAIK, is to share the typeof (unique symbol) between both files, by placing it in a third, so there is only one unique symbol.
(I say typeof because as per above, only the typeof will give you the same reference to the unique symbol, so you can't just share the unique symbol, you have to share the typeof)

Reproduction

Just to remove svelte components entirely, i'll use a .ts file repro.
Place the following in a .ts file in a svelte project (obviously, modify the node_modules path to be correct for your project).

import type { Snippet as SrcSnippet } from '../../../node_modules/svelte/src/index.d.ts';
import type { Snippet } from 'svelte';

let foo: SrcSnippet;
let bar: Snippet;

foo = bar;

You will receive the following error:

 ✗  bun run check
$ svelte-kit sync && svelte-check --tsconfig ./tsconfig.json

====================================
Loading svelte-check in workspace: /Users/dannyb/sources/modeler-svelteflow
Getting Svelte diagnostics...

/Users/dannyb/sources/modeler-svelteflow/src/components/SearchableTreeView/foo.ts:7:1
Error: Type 'import("svelte").Snippet<[]>' is not assignable to type 'import("/Users/dannyb/sources/modeler-svelteflow/node_modules/svelte/src/index").Snippet<[]>'.
  Type '{ '{@render ...} must be called with a Snippet': "import type { Snippet } from 'svelte'"; } & typeof SnippetReturn' is not assignable to type '{ '{@render ...} must be called with a Snippet': "import type { Snippet } from 'svelte'"; } & typeof SnippetReturn'. Two different types with this name exist, but they are unrelated.
    Type '{ '{@render ...} must be called with a Snippet': "import type { Snippet } from 'svelte'"; } & unique symbol' is not assignable to type 'unique symbol'.

foo = bar;

It is correct - the types are in fact unrelated. The strings about @render are a red herring they are just part of the type name in the file.

The complaint that the different unique symbols aren't compatible is the real complaint.

Now, i will say, while it is easy to reproduce a failure and show it's broken - it is hard to track down the exact circumstances under which both end up getting into the picture.

But it's definitely the same failure in the real world (and in the issues i can find on the issue tracker).

In the real world I have a failing treeview where the render is in a child component (treeitem), the snippet is in the parent component (treeview), and the typecheck failure is in the parent - this is the output of svelte check:

Svelte: Type '(label: string, query: string) => ReturnType<import("svelte").Snippet>' is not assignable to type 'Snippet<[string, string]>'.

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

Type '{ '{@render ...} must be called with a Snippet': "import type { Snippet } from 'svelte'"; } & unique symbol' is not assignable to type 'unique symbol'.

If i remove the snippet related declarations from types/index.d.ts, it causes the typecheck failure to go away. So they are definitely getting imported into the language server, and svelte check.

In any case, all of this is somewhat moot - i'm just recording it in case it's ever useful for some other bug later. It should hopefully be obvious this is broken, and there is a hopefully easy fix the overall issue - share the typeof <unique symbol> between the two files so they are the same unique symbol.

I also am going to go over to the typescript repo and file a bug that they should give the unique symbols context info or something in the error messages - so that it is obvious they are not the same.

Logs

System Info

Not relevant

Severity

annoyance

@paoloricciuti
Copy link
Member

I think we need a reproduction for this... who's gonna import a type from node modules? I understand what you are saying but I don't see how this can be an issue in a real project.

@paoloricciuti paoloricciuti added the awaiting submitter needs a reproduction, or clarification label Feb 2, 2025
@dberlin
Copy link
Author

dberlin commented Feb 2, 2025

The repro is obviously contrived to show the result - in previous issues, people assumed it was the setup that was broken and causing a buggy error message.
I just wanted to show that the error message is not buggy, it is real.

In this case - as i said, it happens in my real project, which does nothing abnormal (as far as i know). I have shared @dberlin/modeler-svelteflow with you privately because it's very hard to minimize the code but keep the bug, and it has code i can't make public (ie pro examples from xyflow).

It normally requires the migrate/svelte5 branch of @xyflow/svelte, which makes it annoying to just build.
However, you can just ignore that, run check, ignore the other errors, and you'll still see the failure in SearchableTreeView.svelte.

So it fails even without all the packages installed.

svelte-check does not support the equivalent of generateTrace that tsc/tsserver has to trace import and type resolution, and adding generateTrace to the tsconfig doesn't work for svelte-check, so it's hard to trace the imports to see what is going on. I spent a bit of time seeing if i could hack the tracing support into svelte-check, but no luck - it doesn't appear to be exported from the typescript apis svelte-check uses in a way that makes this easy. That would have been really nice, as it would likely immediately pinpoint how things are breaking.

I also spent a bit of time trying to see what is going on in the svelte-language-server, but i ran out of time. there are a lot of suspects to run down - it has it's own module resolution mechanism and caching, the generated index.d.ts files it use force-import types/index.js, etc.

If i get more time in the next few days i'll try to continue chasing it down.

As for other real world examples - besides the bug reports on the issue tracker, there are other edge cases that will fail, but not sure if anyone cares.

For example - any case where you use a library that vendors svelte 5 or it's types, the snippet types will be incompatible with your non-vendored one. So say a component library that vendors svelte types will not be able to render snippets you pass it from your non-vendored copy.

I will say that i'm also happy to send a PR to just fix this issue. It should cause no harm elsewhere, and they are definitely different types.

@paoloricciuti
Copy link
Member

I just wanted to show that the error message is not buggy, it is real.

It is real...if people were importing the unique symbol from that specific file. That's what I'm saying, obviously you can cause the error but is it really possible without this weird setup?

In this case - as i said, it happens in my real project, which does nothing abnormal (as far as i know).

Which is exactly why we need a minimal repro: as far as you know you are not doing anything weird...but maybe you are and we would chase a bug that is not there.

I have shared @dberlin/modeler-svelteflow with you privately because it's very hard to minimize the code but keep the bug, and it has code i can't make public (ie pro examples from xyflow).

I'll try to take a look if I have time but I can't guarantee...I can't stress enough how important is that the reproduction is minimal. Before being able to figure out if you are doing something wrong I have to explore your whole project which also involves libraries for which I might not even have knowledge of.

For example - any case where you use a library that vendors svelte 5 or it's types, the snippet types will be incompatible with your non-vendored one. So say a component library that vendors svelte types will not be able to render snippets you pass it from your non-vendored copy.

This feels weird to me: svelte library generally are not bundled and the imports are resolved within the user project

@dberlin
Copy link
Author

dberlin commented Feb 2, 2025

"It is real...if people were importing the unique symbol from that specific file. That's what I'm saying, obviously you can cause the error but is it really possible without this weird setup?"

Yes, i've given you one example, #13760 has another example.

I will say i'm a bit confused at what you want - do we want a real world example or a minimal one? Those are very different things. I thought you wanted a real world one, so i gave you a real world one. It sounds like what you really want may be an idiomatic minimal one?

In any case, hopefully, in the interest of moving forward, I tracked down why it happens, at least in this case.

I got a little more time to instrument the module loading in the language service.

The module cache in svelte-language-service is not always resolving the 'svelte' module to the same filename, depending on the paths involved and order of files loaded (I didn't dive too deep on the exact cause of resolution order change). Cache misses then cause it to two load copies of types.

So in one load order, it is resolving the svelte module to a global cache:

Setting module svelte, containing file /Users/dannyb/sources/modeler-svelteflow/node_modules/@xyflow-svelte/packages/svelte/dist/lib/plugins/Controls/types.d.ts, resolved filename is
/Users/dannyb/.pnpm/[email protected]/node_modules/svelte/types/index.d.ts

But later:

Checking for module svelte, containing file /Users/dannyb/sources/modeler-svelteflow/src/routes/apps/+layout.svelte, result is miss
Setting module svelte, containing file /Users/dannyb/sources/modeler-svelteflow/src/routes/apps/+layout.svelte, resolved filename is /Users/dannyb/sources/modeler-svelteflow/node_modules/svelte/types/index.d.ts

In a different file load order, it will do the second, then the first becomes a cache hit instead of a miss, and you get no error.

Note: In this particular case, these are actually the same file (not like content wise, but literally, there are not two files on the filesystem), but it is loading a new instance of the module anyway, causing a new instance of the unique symbol to occur.

So in this particular case, it is loading the types file twice, changing the definition of SnippetReturn as that happens.

But if this was not the case, it looks like you'd end up with two different files loaded.

I can also get it to load the src version and the types version ,through messing with file import order and some of the bare import statements that are around.

Hopefully this is enough for you to go on.

@paoloricciuti
Copy link
Member

Yes, i've given you one example, #13760 has another example.

Did you link to the wrong PR? I saw no error there.

I will say i'm a bit confused at what you want - do we want a real world example or a minimal one? Those are very different things.

Yeah a minimal example that doesn't involve you importing from node modules.

From the resto of your exploration it seems is more of a language tools issue than a svelte one.

@dberlin
Copy link
Author

dberlin commented Feb 2, 2025

Sorry, #13670. There are others as well.

As for the rest - what's here is clearly a bug, whether you decide to fix it or not.
You can decide you don't care for whatever reason you like. That's the joy and pain of running projects.

I'll respect that.
I will only say it seems very strange to me. Even if it does not trigger now, and i'm 100% sure i can make it trigger even under the constraints you ask for, eventually it seems guaranteed to cause a problem, and is totally and completely trivial to fix. It'd be one thing if it was a painful rearchitecture, but this is like a 5 line fix.
I honestly can't think of a time that someone took this approach and it did not end up coming back to haunt them badly :-)
I am also happy to fix it and submit a PR, so that you have no work to do other than review.

But in the end, it is your decision to make.
I'll close this for now, and take it up with language tools since you believe that's where it goes.
If it does come back to bite you, and you decide you need help, i remain happy to help.

@dberlin dberlin closed this as completed Feb 2, 2025
@paoloricciuti
Copy link
Member

What? I never said we should close this... I'm trying to understand the issue so that it can be fixed in the right way. Saying "if the user imports the type from node modules it will trigger the issue" it's weird because none will ever do that.

I'm asking for a minimal reproduction so that if we understand what really cause the error we can fix it there. That's it.

@paoloricciuti paoloricciuti reopened this Feb 2, 2025
@dberlin
Copy link
Author

dberlin commented Feb 2, 2025

That's not what I said, anywhere, so not sure why you put that in quotes. In fact, i specifically said that people in the past, like in #13670, simply dismissed it as not a real error. I also said i will give you a a very small reproduction to show you it's a real error. Which i did. I specifically said the same error occurs in my real world project, and pasted the error message right after the example. Then after you asked for the real world project, i gave you that too.

What i've actually said other than that, is that it doesn't matter how it happens. It only matters that multiple instances of either type occur.
That is the really causes it.
It does not matter if a user does it, or the language checker does it, or anything does it.
It does not matter if you load the same file twice, load both files, or load different copies of the same file, or different otherwise compatible copies of the file, or whatever.
I've shown you lots of ways it can happen - real world, contrived, language checker, svelte's own code.
Hunting down every path through which this can happen seems .. kinda pointless - there are a ton of them, and unless you are going to declare every single path to be user error, it won't get you any closer to fixing this.

We are clearly not communicating effectively to say the least, and this seems highly unproductive at this point, so i'm going to bow out of this bug.

@envyvox
Copy link

envyvox commented Feb 2, 2025

Hey, got same error on "real project".
Image

@paoloricciuti
Copy link
Member

That's not what I said, anywhere, so not sure why you put that in quotes. In fact, i specifically said that people in the past, like in #13670, simply dismissed it as not a real error. I also said i will give you a a very small reproduction to show you it's a real error. Which i did. I specifically said the same error occurs in my real world project, and pasted the error message right after the example. Then after you asked for the real world project, i gave you that too.

What I meant with my interaction is that saying "yeah previously was real" without providing a series of step to reproduce is not helpful to understand the error. Neither is saying I should import from node modules to reproduce. That's why I asked you for a real minimal reproduction which you said you could provide so I was waiting for you to do it. Since this error can happen in situations that are not bugs it's important to get a reproduction not just say "it can happen". I said I will take a look at your non minimal reproduction but I don't know when I can get to it and if you provide a minimal one I'll take a look immediately.

What i've actually said other than that, is that it doesn't matter how it happens. It only matters that multiple instances of either type occur.
That is the really causes it.
It does not matter if a user does it, or the language checker does it, or anything does it.
It does not matter if you load the same file twice, load both files, or load different copies of the same file, or different otherwise compatible copies of the file, or whatever.
I've shown you lots of ways it can happen - real world, contrived, language checker, svelte's own code.
Hunting down every path through which this can happen seems .. kinda pointless - there are a ton of them, and unless you are going to declare every single path to be user error, it won't get you any closer to fixing this.

You described how it could happen...what I'm looking at is a series of step I can follow to make it happen which I've yet to see and that's all I'm asking for.

@paoloricciuti
Copy link
Member

Hey, got same error on "real project". Image

Perfect...can you show me what prefix is? How can I create a similar component so I can test what's going on?

@envyvox
Copy link

envyvox commented Feb 2, 2025

Hey, got same error on "real project". Image

Perfect...can you show me what prefix is? How can I create a similar component so I can test what's going on?

It's kinda compicated to replicate...
So, prefix is a prop of component that comes from npm package

We have a separate project for components and other stuff that being published on private npm.
d.ts looks like this, and they ofc comes from node_mobules

Image

@paoloricciuti
Copy link
Member

Hey, got same error on "real project". Image

Perfect...can you show me what prefix is? How can I create a similar component so I can test what's going on?

It's kinda compicated to replicate... So, prefix is a prop of component that comes from npm package

We have a separate project for components and other stuff that being published on private npm. d.ts looks like this, and they ofc comes from node_mobules

Image

So you are getting that error in the "components project" or where you use them?

@envyvox
Copy link

envyvox commented Feb 2, 2025

So you are getting that error in the "components project" or where you use them?

Only where we are using them.

@paoloricciuti
Copy link
Member

So you are getting that error in the "components project" or where you use them?

Only where we are using them.

And how you get access to prefix? Is a snippet argument? Also I can't see how you are using it. If it's a snippet shouldn't it be rendered instead of just used as an expression?

@envyvox
Copy link

envyvox commented Feb 2, 2025

So you are getting that error in the "components project" or where you use them?

Only where we are using them.

And how you get access to prefix? Is a snippet argument? Also I can't see how you are using it. If it's a snippet shouldn't it be rendered instead of just used as an expression?

OH mb, nvm, we are getting that error on both projects...

Image

@paoloricciuti
Copy link
Member

So you are getting that error in the "components project" or where you use them?

Only where we are using them.

And how you get access to prefix? Is a snippet argument? Also I can't see how you are using it. If it's a snippet shouldn't it be rendered instead of just used as an expression?

OH mb, nvm, we are getting that error on both projects...

Image

Thanks...could you please copy your .d.ts here so I can try to "use them" and see what's going on?

@envyvox
Copy link

envyvox commented Feb 2, 2025

Thanks...could you please copy your .d.ts here so I can try to "use them" and see what's going on?

Yeah, just in case i'll copy them "as is" without removing stuff

Input.svelte.d.ts

import type { InputProps } from "../types.js";
declare const Input: import("svelte").Component<InputProps, {}, "value">;
type Input = ReturnType<typeof Input>;
export default Input;

types.d.ts

import type { Snippet } from "svelte";
import type { HTMLAttributes, HTMLInputTypeAttribute } from "svelte/elements";
import type { inputSizes } from "./consts.js";
export type InputSize = (typeof inputSizes)[number];
export type InputProps = {
    value?: string;
    error?: string;
    placeholder?: string;
    isSuccess?: boolean;
    size: InputSize; 
    className?: string;
    disabled?: boolean;
    prefix?: Snippet;
    suffix?: Snippet;
    type?: HTMLInputTypeAttribute;
    name?: string;
} & HTMLAttributes<HTMLInputElement>;
export type InputErrorLabelProps = {
    value?: string | number;
    error?: string;
    className?: string;
};
export type InputPlaceholderLabelProps = {
    value?: string | number;
    placeholder?: string;
    size: InputSize;
    className?: string;
};

consts.d.ts

export declare const inputSizes: readonly ["large", "medium", "small"];

@envyvox
Copy link

envyvox commented Feb 2, 2025

Hey, @paoloricciuti , sry for pinging, but I actually found what was causing problem for me:

export type InputProps = {
  prefix?: Snippet
} & HTMLAttributes<HTMLInputElement>

Removing & HTMLAttributes<HTMLInputElement> from type solves error, as well as renaming prefix to something else

@paoloricciuti
Copy link
Member

Uh this together with @dberlin explanation might be a very good hint...maybe the html attributes type is importing from the different symbol? I can't explore this right now but I'll try tomorrow

@envyvox
Copy link

envyvox commented Feb 3, 2025

I just found out that HTMLAttributes does have prefix atrribute that have type string, which makes my error completely legitimate and correct,

I apologize that I may have led your thoughts in the wrong direction in solving this issue...

I've just never seen or heard of its existence...

@paoloricciuti
Copy link
Member

I just found out that HTMLAttributes does have prefix atrribute that have type string, which makes my error completely legitimate and correct,

I apologize that I may have led your thoughts in the wrong direction in solving this issue...

I've just never seen or heard of its existence...

No worries...I wonder if it could be a similar problem for @dberlin

@paoloricciuti
Copy link
Member

@dberlin i went take a look at your code but i don't have access anymore, if you could invite me again i have a bit of time now

@webJose
Copy link
Contributor

webJose commented Feb 3, 2025

I see that this has had a long conversation. I won't butt-in, but I'll mention that I opened #13759 some time ago regarding problems with the Snippet type. Mine has a simple code, I would say.

@paoloricciuti
Copy link
Member

I see that this has had a long conversation. I won't butt-in, but I'll mention that I opened #13759 some time ago regarding problems with the Snippet type. Mine has a simple code, I would say.

Nice I'll try to take a look

@MrBns
Copy link

MrBns commented Feb 5, 2025

Wow.. I am not alone.
getting same error here. Lol. its really weird.

Screenshot of Error
Image

and here is the code my component.

SectionBigSlider.svelte

{#snippet sliderTopTitle({ direction, title }: TSliderTopTitleParam)}
	<div class="title-wrapper relative h-[100px] lg:h-[300px]">
		<h1
			class="text-h3 lg:text-h2 2xl:text-h1 absolute top-1/2 w-full -translate-y-1/2 {direction === 'left'
				? 'text-shark-500 right-10 text-end'
				: direction === 'right'
					? 'text-shark-500 left-10 text-start'
					: 'text-center'}"
		>
			{@html title}
		</h1>
	</div>
{/snippet}
<!--Passing snippet to component  -->
	<section class="">
		<div class="ml:h-[1040px]">
			<swiper-container pagination="true" pagination-clickable={true} class="big-slider-container ml:h-[1040px]">
				<swiper-slide>
					<div class="container h-full">
						<BigSliderSlide1 {sliderTopTitle} />
					</div>
				</swiper-slide>
				
			</swiper-container>
		</div>
	</section>

BigSlider1.svelte (just props type)

let {
		sliderTopTitle
	}: {
		sliderTopTitle: Snippet<[TSliderTopTitleParam]>;
	} = $props();

I got same issue a month ago. but random work arround did solve it. most probably its a dependency mismatch issue.

My Dependencies

    "@eslint/compat": "^1.2.5",
    "@sveltejs/adapter-auto": "^3.0.0",
    "@sveltejs/adapter-node": "^5.2.12",
    "@sveltejs/kit": "^2.17.1",
    "@sveltejs/vite-plugin-svelte": "^5.0.3",
    "@tailwindcss/vite": "^4.0.3",
    "dayjs": "^1.11.13",
    "eslint": "^9.7.0",
    "eslint-config-prettier": "^9.1.0",
    "eslint-plugin-svelte": "^2.46.1",
    "globals": "^15.14.0",
    "postcss": "^8.5.1",
    "prettier": "^3.4.2",
    "prettier-plugin-svelte": "^3.3.3",
    "prettier-plugin-tailwindcss": "^0.6.11",
    "sass": "^1.83.4",
    "svelte": "^5.19.8",
    "svelte-check": "^4.1.4",
    "tailwindcss": "^4.0.3",
    "typescript": "^5.7.2",
    "typescript-eslint": "^8.19.0",
    "vite": "^6.0.11"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification
Projects
None yet
Development

No branches or pull requests

5 participants