Add better Typescript support for modals

[YummYume]

Describe what feature you'd like. Pseudo-code, mockups, or screenshots of similar solutions are encouraged!

Hello. Currently working with Skeleton again on a new project, I am thrilled to see how well it has grown and how amazing it is (it just works). However, I feel Typescript is currently not very well supported for some resources and is crucial for a good dev experience.

Let's take an example with a simple custom modal, which I have mostly just copy-pasted from the docs :

<script lang="ts">
    import { modalStore, type ModalComponent, type ModalSettings } from '@brainandbones/skeleton';
    import Modal from '$components/Modal.svelte';

    function triggerCustomModal(): void {
        const modalComponent: ModalComponent = {
            ref: Modal,
            props: { background: 'bg-red-500' }
        };
        const d: ModalSettings = {
            type: 'component',
            component: modalComponent
        };

        // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call
        modalStore.trigger(d);
    }
</script>

<button on:click={triggerCustomModal}>Modal</button>

// Modal.svelte
<script lang="ts">
    export let parent: Record<string, any>;
</script>

<h1>Test</h1>

<button class="btn" on:click={parent.onClose}>Close</button>

Here we already have a few issues, such as modalStore being typed as any, requiring disabling eslint while not providing any IntelliSense.

In my Modal.svelte, I cannot access my props with typing (although that's not the most annoying), but I also have to type the parent prop as either any or Record to avoid having errors on my parent type (like when calling parent.onClose). And again, it does not provide any IntelliSense, and I feel like it really should.

I have not yet tested all of the different components available, so I can't say for sure about the status of Typescript support for the rest of this library.

What type of pull request would this be?

Enhancement

Any links to similar examples or other references we should review?

If we take for example a SvelteKit app with a +page.server.ts page where I export some data, then I can fully access them in my +page.svelte by simply typing my data prop with PageData.

Taken directly from the SvelteKit doc

<script lang="ts">
  import type { PageData } from './$types';

  export let data: PageData;
</script>

Although this is just an example of how it could look, I still feel like some parts need improvement to provide better Typescript support and provide at least a basic IntelliSense.

[endigo9740]

Thanks for bringing this to my attention @YummYume, yeah the Modal store was not quite up to par with Toasts and our upcoming Drawer updates. I've created this PR if you wish to test my improvements:

• Improve TS handling for Modals #623

Unfortunately we don't have a quick way to create a package tarball or allow you to install from a dev tag from NPM yet, so in the meantime I might recommend cloning down the Skeleton project itself and testing anywhere in the app. Create a new page or test directly on the existing Modals documentation page in:

Routes > Inner > Utilities > Modals > +page.svelte

But I'm seeing much improved IntelliSense here:

I'll leave the PR open for a day or so to give you a chance to review. But otherwise I'd expect this to be merged and be part of the next release in about a week and a half.

[YummYume]

Hey @endigo9740, thanks for such a quick response (and work)! I've played around a bit with the modalStore and I think everything is here, this is great! 🚀

However, I still think the parent prop for custom Modal components should get a proper type to avoid dirty workarounds. With a type like ModalParent we could access all the props available here. Other additional props are easy enough to type so it shouldn't be a problem?

<script lang="ts">
  export let parent: ModalParent;
  export let customAdditionalProp: string;
</script>

[endigo9740]

@YummYume awesome glad to hear.

Oh and thanks for reminding me about the parent prop, I meant to explain that. So yes, absolutely we can look to improve that now. But currently the way it's working is (hopefully) temporary as its very maintenance heavy. Essentially Svelte/Kit offers up a way to query all available parent prop values, however the API for this is not stable. Due to this if a prop is added or change in the parent, we have to add it to a separate object, and that object is then passed down to the child. Not the end of the world, but certainly leaves room for error. Adding a type would mean that we're now maintaining the props, the object, and now a new type that have to have all these changes stay in sync. So it does give me some pause.

I'll review going ahead and setting it up though. But does seem fragile and prone to errors for future updates. But we'll see!

[endigo9740]

@YummYume I feel like what we have now is a good step forward so I'm going to go ahead and merge this. It'll be part of the next release of course. Still trying to decide how I want to proceed on the parent props, but may be something we circle back to in the future. Suggestions are welcome!