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

TypeScript issue with flexrender for svelte-table examples #4778

Closed
2 tasks done
tiptenbrink opened this issue Mar 27, 2023 · 15 comments · Fixed by #4932
Closed
2 tasks done

TypeScript issue with flexrender for svelte-table examples #4778

tiptenbrink opened this issue Mar 27, 2023 · 15 comments · Fixed by #4932

Comments

@tiptenbrink
Copy link

Describe the bug

On the newest version of TypeScript (5.0.2) and Svelte (3.57.0), running the examples, for instance the "Sorting" example for svelte-table, there is a TypeScript error that I am unable to wrap my head around. It occurs for every flexRender inside a dynamic Svelte component (svelte:component).

Inside the dynamic SvelteKit component, the following TypeScript error occurs, for instance the main table body:

{#each row.getVisibleCells() as cell}
  <td>
    <svelte:component this={flexRender(cell.column.columnDef.cell, cell.getContext())} />
  </td>
{/each}
Argument of type '{ render: (props?: {}, { $$slots, context }?: { $$slots?: {}; context?: Map<any, any>; }) => { html: any; css: { code: string; map: any; }; head: string; }; $$render: (result: any, props: any, bindings: any, slots: any, context: any) => any; } | (new (options: any) => { ...; })' is not assignable to parameter of type 'ConstructorOfATypedSvelteComponent'.
  Type '{ render: (props?: {}, { $$slots, context }?: { $$slots?: {}; context?: Map<any, any>; }) => { html: any; css: { code: string; map: any; }; head: string; }; $$render: (result: any, props: any, bindings: any, slots: any, context: any) => any; }' is not assignable to type 'ConstructorOfATypedSvelteComponent'.

Possible causes:
- You use the instance type of a component where you should use the constructor type
- Type definitions are missing for this Svelte Component. If you are using Svelte 3.31+, use SvelteComponentTyped to add a definition:
  import type { SvelteComponentTyped } from "svelte";
  class ComponentName extends SvelteComponentTyped<{propertyName: string;}> {}

It seems like the flexRender doesn't have all the typing information that Svelte wants.

Note: everything still works as required, the table renders without errors. This seems to be a TypeScript-only issue.

Any help or primers on how to begin solving this would be appreciated.

Your minimal, reproducible example

https://codesandbox.io/p/sandbox/happy-leaf-eyudi0?file=%2Fsrc%2FApp.svelte

Steps to reproduce

  1. Go to above Sandbox (or just open the existing Sorting example and ensure Svelte and TypeScript are on their most recent version)
  2. Run it in VS Code for full TypeScript support
  3. Observe the error in the editor

Expected behavior

No error is shown, as the underlying code seems to work (the table renders).

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

  • Linux Ubuntu 22.04, VS Code 1.76.2

react-table version

8.8.4

TypeScript version

5.0.2

Additional context

No response

Terms & Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.
@thenbe
Copy link
Contributor

thenbe commented Mar 30, 2023

It seems this was introduced in 8.8.3 by #4588.

As a workaround, you can pin @tanstack/table to version 8.8.2 until this issue is closed.

@benbender
Copy link

Seeing the same issue, but even pinning to 8.8.2 doesn't resolve the issue.

@itssumitrai
Copy link
Contributor

If your flexRender is rendering a component in the result, then its going to output an instance of the component.
like:

<Hello foo="bar" />

so in order to fix your types, you can use your flexRender like simply

{flexRender(cell.column.columnDef.cell, cell.getContext())}

instead of

<svelte:component this={flexRender(cell.column.columnDef.cell, cell.getContext())} />

svelte:component can handle both instance & constructor but seems like type only wants a constructor in there.

@Eric-Arz
Copy link

Still not resolved. How can something like this happen? Don't you guys even test if your own basic example is still fine after changing stuff?

@philmas
Copy link

philmas commented Apr 16, 2023

If your flexRender is rendering a component in the result, then its going to output an instance of the component. like:

<Hello foo="bar" />

so in order to fix your types, you can use your flexRender like simply

{flexRender(cell.column.columnDef.cell, cell.getContext())}

instead of

<svelte:component this={flexRender(cell.column.columnDef.cell, cell.getContext())} />

svelte:component can handle both instance & constructor but seems like type only wants a constructor in there.

Your suggestions appears to fix the typing issue but it outputs raw code instead of the component and so is not just a replacement solution. Perhaps I am doing something wrong.

@benbender
Copy link

To whom it may concern, here is my temporary solution:

import type { ComponentType, SvelteComponentTyped } from "svelte";
import { flexRender as flexRenderOrig  } from "@tanstack/svelte-table";

const flexRender = <P extends Record<string, any>, C = any>(
    component: C,
    props: P
  ): ComponentType<SvelteComponentTyped> =>
    flexRenderOrig(component, props) as ComponentType<SvelteComponentTyped>;

Usage:

<svelte:component
  this={flexRender(
    header.column.columnDef.footer,
    header.getContext()
  )}
/>

@thenbe
Copy link
Contributor

thenbe commented Apr 21, 2023

svelte:component can handle both instance & constructor but seems like type only wants a constructor in there.

@itssumitrai Besides the typing issue, would you happen to know if one approach is better than the other (in terms of performance, behavior, etc.)? Or are they mostly identical?

@sjunepark
Copy link

@benbender Thanks, the solution worked for now. Hope this gets fixed.

@KevinVandy
Copy link
Member

@benbender Thanks, the solution worked for now. Hope this gets fixed.

We will accept a PR for a fix

@LukePeltier
Copy link

I believe this ticket needs to be reopened. The commit and pull request mentioned is not actually merged into main

@sukeshpabolu
Copy link

Sir @tannerlinsley , please look into this. :).

@nadanke
Copy link

nadanke commented Aug 17, 2023

I'm pretty sure this has worked in the meantime but getting the same error now again, as well

@LukePeltier
Copy link

@KevinVandy Do you have any knowledge of what happened here?

@KevinVandy
Copy link
Member

Looks like there was a problem last month where a push erased some git history. The last few prs merged are missing in the main branch

@KevinVandy
Copy link
Member

Re-released in 8.9.6

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

Successfully merging a pull request may close this issue.