Skip to content

fix: component type declaration #5

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
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

brandodo
Copy link

  • Add component typing for Svelte 5

@theetrain
Copy link
Owner

Thanks, Brandon.

While testing your changes in a Svelte 4 project, I noticed the Cartesian's Component prop resolves as type any since Component is not an export in Svelte 4.

image

Without these changes, what's the impact? In a Svelte 5 project, I don't see deprecation warnings for ComponentType even though it is deprecated.

image

I can also help add feature parity to CartesianWithRunes that way you can upgrade to the newer component with newer types. I only need to resolve what's left in #1, and there shouldn't be breaking changes; that way you can upgrade incrementally.

@brandodo
Copy link
Author

@theetrain getting this error when working with a component using Svelte 5 syntax:

Error: Type 'Component<Props, {}, "">' is not assignable to type 'ComponentType'.
  Type 'Component<Props, {}, "">' is not assignable to type 'new (options: ComponentConstructorOptions<Record<string, any>>) => SvelteComponent<Record<string, any>, any, any>'.
    Type 'Component<Props, {}, "">' provides no match for the signature 'new (options: ComponentConstructorOptions<Record<string, any>>): SvelteComponent<Record<string, any>, any, any>'. (js)

<Cartesian {props} Component={Switch} />

@theetrain
Copy link
Owner

theetrain commented Apr 25, 2025

Please share a full code example showing how props are defined for this "Switch" example. Also does the error occur in your editor, at build time, or someplace else?

@brandodo
Copy link
Author

Occurs during svelte-check run

<script>
  /**
   * @typedef Props
   * @property {string} label
   * @property {'off' | 'on'} toggle
   * @property {boolean} asyncSwitch
   * @property {string} formaction
   * @property {import('svelte/elements').HTMLAttributes<HTMLSpanElement>} sliderWrapperSpanAttributes
   * @property {import('svelte/elements').HTMLAttributes<HTMLSpanElement>} sliderSpanAttributes
   * @property {string} [id=crypto.getRandomValues(new Uint32Array(1))[0]]  A unique ID for the input. This defaults to the value of the `name` prop
   * @property {'sm' | 'md'} [size='md'] The size of the input. Using `small` will set the padding of the input to `8px 16px` and `medium` will set the padding of the input to `12px 16px`.
   * @property {import('svelte/elements').HTMLButtonAttributes} buttonAttributes
   * @property {HTMLElement} ref
   */

  /** @type {Props} */
  let {
    label,
    toggle,
    asyncSwitch,
    formaction,
    sliderWrapperSpanAttributes,
    sliderSpanAttributes,
    id,
    size,
    buttonAttributes,
    ref
  } = $props()
</script>

- Use `Component` type when Svelte 5 is present, otherwise fallback to `ComponentType`
- Include Svelte 5 in `peerDependencies`
- Patch dependencies
- HTML div attributes threw false positives that required overrides.

Details: sveltejs/svelte#13959 (comment)
@theetrain
Copy link
Owner

Hi Brandon,

I pushed some additional fixes to your branch, please try it out in your project:

  1. Pull latest changes in your svelte-cartesian fork
  2. Run npm pack
  3. Install the tarball in your project
  4. Observe type checker (you may need to restart the Svelte language server after installing)

Summary of my changes:

  1. Add Svelte ^5.0.0 to peerDependencies for npm compatibility.
  2. Update playwright snapshots
  3. Conditionally assign Component type to Cartesian's Component prop when Svelte 5 is installed; otherwise assign ComponentType type to Component prop.

References:

@theetrain theetrain self-requested a review April 28, 2025 16:55
@brandodo
Copy link
Author

@theetrain Tried the steps outlined, however, am still getting type error. Condition for the type seems to evaluate to the truthy result.

@theetrain
Copy link
Owner

theetrain commented Apr 30, 2025

What are your installed package versions for the following?

npm ls svelte svelte-check typescript svelte-cartesian

Also please share a screenshot of the error.

While I investigate this and follow up, you can temporarily install your commit as a workaround if it unblocks you:

npm i -D git://github.com/theetrain/svelte-cartesian.git#d9c3edbd21f66d4ed3ac0683ae9f8551e039dd51

@brandodo
Copy link
Author

brandodo commented May 1, 2025

@theetrain

@viralnation/[email protected] /Users/bong/Desktop/viralnation/ui-svelte
├─┬ @storybook/[email protected]
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ └── [email protected] deduped
├─┬ @storybook/[email protected]
│ ├─┬ [email protected]
│ │ ├── [email protected] deduped
│ │ └── [email protected] deduped
│ ├── [email protected] deduped
│ ├─┬ [email protected]
│ │ ├── [email protected] deduped
│ │ └── [email protected] deduped
│ └── [email protected]
├─┬ @storybook/[email protected]
│ └── [email protected] deduped
├─┬ @sveltejs/[email protected]
│ ├─┬ @sveltejs/[email protected]
│ │ └── [email protected] deduped
│ └── [email protected] deduped
├─┬ @testing-library/[email protected]
│ └── [email protected] deduped
├─┬ [email protected]
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ └── [email protected] deduped
├─┬ [email protected]
│ └── [email protected] deduped
├─┬ [email protected]
│ └── [email protected] deduped
├─┬ [email protected]
│ ├── [email protected] deduped
│ └── [email protected] deduped
└── [email protected]

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 this pull request may close these issues.

2 participants