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

Generic resolution issue with bind:this/svelte:element #1913

Open
brunnerh opened this issue Mar 3, 2023 · 4 comments
Open

Generic resolution issue with bind:this/svelte:element #1913

brunnerh opened this issue Mar 3, 2023 · 4 comments

Comments

@brunnerh
Copy link
Member

brunnerh commented Mar 3, 2023

Describe the bug

Ran into issues trying to create a generic component that is either a button or an a.

The type is supposed to switch based on whether a href property is set, but there are two problems:

  • bind:this will not resolve a type alias that switches between HTMLButtonElement and HTMLAnchorElement, giving an assignment error.
  • When said bound element is exposed as a property, when using the component the type is not narrowed when the href is not defined, only if it is.

Reproduction

Component definition:

<script lang="ts">
    type Href = $$Generic<string | undefined>;
    type Ref = Href extends string ? HTMLAnchorElement : HTMLButtonElement;

    export let ref: Ref = undefined as any;
    export let type: 'button' | 'submit' | 'reset' = 'button';

    export let disabled = false;
    
    export let href: Href = undefined as any;
    export let target: string | undefined = undefined;

    let tag: 'button' | 'a';
    $: tag = href == null ? 'button' : 'a';
</script>

<svelte:element this={tag} bind:this={ref}
        type={tag === 'button' ? type : undefined}
        href={disabled ? undefined : href}
        target={disabled ? undefined : target}
        disabled={tag === 'button' ? disabled : undefined}
        data-disabled={tag === 'a' ? disabled : undefined}
        {...$$restProps}
        on:click on:focus on:blur >
    <slot />
</svelte:element>

Error on bind:this={ref}:

Type 'HTMLAnchorElement | HTMLButtonElement' is not assignable to type 'Ref'.
  Type 'HTMLAnchorElement' is not assignable to type 'Ref'.ts(2322)

Component usage:

<script lang="ts">
	import Button from './button.svelte';

	let button: HTMLButtonElement;
	let link: HTMLAnchorElement;
</script>

<Button bind:ref={button}>Button</Button> <!-- OK - No error-->
<Button bind:ref={link}>Button</Button> <!-- Wrong - No error -->

<Button bind:ref={link} href="/">Link</Button> <!-- OK - No error -->
<Button bind:ref={button} href="/">Link</Button> <!-- OK - Error -->

For the components with href, the type of ref is resolved correctly, but for the components without, it is HTMLButtonElement | HTMLAnchorElement | undefined resulting in no error from a binding to a variable of the wrong type.

Expected behaviour

  • No error on bind:ref in the component definition
  • Type of element property inferred correctly based on presence of href attribute, leading type errors where appropriate

System Info

  • OS: Windows 11, 64bit
  • IDE: VSCode

Which package is the issue about?

  • Svelte for VS Code extension
  • svelte-check

Additional Information, eg. Screenshots

No response

@brunnerh brunnerh added the bug Something isn't working label Mar 3, 2023
@jasonlyu123 jasonlyu123 removed the bug Something isn't working label Mar 6, 2023
@jasonlyu123
Copy link
Member

jasonlyu123 commented Mar 6, 2023

About the bind:this, I think the easiest way is to use $$Props and let ref as HTMLButtonElement | HTMLAnchorElement. In your code, The condition returns a union type, and it's not the same as the conditional type. And I can't find any equivalent TypeScript that could make it work.

And about the generic, Your generic is inferred as string | undefined instead of undefined. I am not sure how TypeScript inferred the generic. But it should work if you make Ref part of the generic nvm it doesn't work

<script lang="ts">
    type Href = $$Generic<string | undefined>;
    type Ref = Href extends string ? HTMLAnchorElement : HTMLButtonElement;

    type $$Props = {
        ref?: Ref;
        type?: 'button' | 'submit' | 'reset';
        disabled?: boolean;
        href?: Href;
        target?: string | undefined;
    };

    export let ref: HTMLAnchorElement | HTMLButtonElement = undefined as any;
    export let type: 'button' | 'submit' | 'reset' = 'button';

    export let disabled = false;
    
    export let href: Href = undefined as any;
    export let target: string | undefined = undefined;

    let tag: 'button' | 'a';
    $: tag = href == null ? 'button' : 'a';
</script>

<svelte:element this={tag} bind:this={ref}
        type={tag === 'button' ? type : undefined}
        href={disabled ? undefined : href}
        target={disabled ? undefined : target}
        disabled={tag === 'button' ? disabled : undefined}
        data-disabled={tag === 'a' ? disabled : undefined}
        {...$$restProps}
        on:click on:focus on:blur >
    <slot />
</svelte:element>

@brunnerh
Copy link
Member Author

brunnerh commented Mar 6, 2023

I suspected that I could make the bind:this work using $$Props, but for larger components with many properties, this creates a huge amount of duplication.

If there were an exported props type as I suggested here, that could be manipulated instead (e.g. with utility types like Pick and Omit).

@jasonlyu123
Copy link
Member

Find one that works with the generic. According to TypeScript docs this should allow the generic to be not distributive https://www.typescriptlang.org/docs/handbook/2/conditional-types.html#distributive-conditional-types

type Ref = [Href] extends [string] ? HTMLAnchorElement : HTMLButtonElement

Don't think the exported props type will help here. Using $$Props works here because the implementation is union-type, and the $$Props is conditional. If you don't want to use $$Props here, The only solution I can think of is to create another variable and type assert it.

<script lang="ts">
    export let ref: Ref = undefined as any
    let refInner: HTMLAnchorElement | HTMLButtonElement
    
    $: ref = refInner as Ref;
 </script>
 
 <svelte:element this={tag} bind:this={refInner}

Since there is no TypeScript equivalent that can work, I don't think there is much we can do about it.

@brunnerh
Copy link
Member Author

brunnerh commented Mar 6, 2023

I tried to create a $$Props type with manually defined $$ExportedProps as a proof of concept, hoping that it would allow to specify the relationships between the properties without using generics, but currently that results in type errors as well. E.g.

// This would be generated
type $$ExportedProps = {
    ref?: HTMLAnchorElement | HTMLButtonElement;
    href?: string | undefined;
    target?: string | undefined;
    type?: 'button' | 'submit' | 'reset';
    disabled?: boolean;
};

// This would have to added manually
type $$Props = Omit<$$ExportedProps, 'ref' | 'href'> & (
    { href: string; ref?: HTMLAnchorElement } |
    { href: undefined; ref?: HTMLButtonElement }
);

export let ref: HTMLAnchorElement | HTMLButtonElement = undefined as any;
export let type: 'button' | 'submit' | 'reset' = 'button';

export let disabled = false;

export let href: string | undefined = undefined;
export let target: string | undefined = undefined;

let tag: 'button' | 'a';
$: tag = href == null ? 'button' : 'a';

Error:

Argument of type '{ ref: HTMLAnchorElement | HTMLButtonElement; type: "button" | "submit" | "reset"; disabled: boolean; href: string | undefined; target: string | undefined; }' is not assignable to parameter of type '$$Props'.
  Type '{ ref: HTMLAnchorElement | HTMLButtonElement; type: "button" | "submit" | "reset"; disabled: boolean; href: string | undefined; target: string | undefined; }' is not assignable to type 'Omit<$$ExportedProps, "ref" | "href"> & { href: undefined; ref?: HTMLButtonElement | undefined; }'.
    Type '{ ref: HTMLAnchorElement | HTMLButtonElement; type: "button" | "submit" | "reset"; disabled: boolean; href: string | undefined; target: string | undefined; }' is not assignable to type '{ href: undefined; ref?: HTMLButtonElement | undefined; }'.
      Types of property 'href' are incompatible.
        Type 'string | undefined' is not assignable to type 'undefined'.
          Type 'string' is not assignable to type 'undefined'.ts(2345)

This is an issue I have noticed before, where the tooling tries to reconcile the exported props with the custom $$Props definition which cannot always work if there are dependencies between properties. This kind of defeats the purpose of having the ability to define $$Props manually.


Using $: ref = refInner as Ref; is a workaround I am currently employing to resolve the bind:this issue.

The other one (when using the component and binding ref) is still unresolved though, and there might not be a workaround for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants