Uncaught error when using custom elements in toast.promise()

[MarcusCemes]

Description

The loading property accepts a custom Svelte element, however success and error do not.

toast.promise(promise, {
    loading: Toast,
    success: Toast,
    error: "I failed!",
});

A custom success element will throw an error which is caught, and the failed state is rendered.

The following error is thrown (not visible in the REPL):

Uncaught (in promise) TypeError: Class constructors cannot be invoked without 'new'
    at resolveValue (types.js?v=72f77d3a:2:82)
    at toast.js?v=72f77d3a:43:21

which seems to map to this line:

https://github.com/kbrgl/svelte-french-toast/blob/533d5800519684fcef73f18d9024b6177ebc015f/src/lib/core/types.ts#L29

This may be due to the fact that success and error are calling resolveValue(), and loading isn't?

https://github.com/kbrgl/svelte-french-toast/blob/533d5800519684fcef73f18d9024b6177ebc015f/src/lib/core/toast.ts#L60-L64

Reproduction

Svelte REPL

[MarcusCemes]

After reading the source code and trying to understand why resolveValue is called, it seems that if success or error is a function, it's called with a single argument, the promise's resolved value, allowing for a dynamic result message based on the resolved/rejected value.

As a Svelte component seems to be a class, it is mistaken for a function. The following works as intended:

toast.promise(new Promise((res) => setTimeout(res, 1000)), {
    loading: Toast,
    success: (_) => Toast,
    error: "I failed!",
});

However, it is inconsistent with the types, as () => SvelteComponent does not match the Renderable signature.

I see two solutions:

1. Check if the value is a class (instanceof? prototype analysis?), assume it's a SvelteComponent and use the new keyword
2. Update the success and error fields to something like type IntoRenderable = ((value: T) => Renderable) | Renderable?

[kbrgl]

Thanks for opening the issue (and your thorough investigation!). This seems to be a place where I made a mistake porting React Hot Toast. Personally, I like the second option you presented better. (If we assume all classes passed in are SvelteComponents, it limits our ability to extend the API in the future, and prototype analysis seems inelegant.)

[Yiak]

Yeah, I can confirm TS also angry with

{
    loading: 'Sending...',
    success: 'Message sent successfully!',
    error: (rej) => `Error:${rej}`
}

Type '(rej: any) => string' is not assignable to type 'Renderable'.ts(2322)

14 should fix this.

[Yiak]

@kbrgl Hi, can you make a NPM release for this update?

[kbrgl]

@Yiak Sure! I can't figure out if this should be a patch or major release according to semver, though. The change is potentially breaking: if you passed a Renderable in, you will now get a type error. On the other hand, if you were passing a Renderable in, you were likely getting a runtime error.

Any thoughts? Short of any comments on this, I'll probably make it a patch release since from a correctness POV you probably want a type error in place of a runtime error. (Otherwise, I don't imagine you'd be using TypeScript.)

[MarcusCemes]

Just my two cents, I see this more as a patch as the types should now match the correct documented/runtime behaviour. I don't think TypeScript users were able to use the component API without suppressing type errors anyway?

[Yiak]

I have made another pull request for this issue. It followed the upstream solution so there should be no runtime errors or type errors. 😀