sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.87k stars 4.14k forks source link

Declare type for the options parameter of the mount() function on its own #13655

Open webJose opened 2 days ago

webJose commented 2 days ago

Describe the problem

For declaring ambient modules and even regular types for code that uses mount() and would like to type things, the type for the options parameter is not named, and instead is defined inline:

 * @param {{} extends Props ? {
 *      target: Document | Element | ShadowRoot;
 *      anchor?: Node;
 *      props?: Props;
 *      events?: Record<string, (e: any) => any>;
 *      context?: Map<any, any>;
 *      intro?: boolean;
 *  }: {
 *      target: Document | Element | ShadowRoot;
 *      props: Props;
 *      anchor?: Node;
 *      events?: Record<string, (e: any) => any>;
 *      context?: Map<any, any>;
 *      intro?: boolean;
 *  }} options

Describe the proposed solution

Declare the options parameter's type as its own type:

 export type MountOptions<Props extends Record<string, any> = Record<string, any>> = {} extends Props ? {
        target: Document | Element | ShadowRoot;
        anchor?: Node;
        props?: Props;
        events?: Record<string, (e: any) => any>;
        context?: Map<any, any>;
        intro?: boolean;
    }: {
        target: Document | Element | ShadowRoot;
        props: Props;
        anchor?: Node;
        events?: Record<string, (e: any) => any>;
        context?: Map<any, any>;
        intro?: boolean;
    }

BTW, now that I see the type, maybe the following is more succinct?

export type MountOptions<Props extends Record<string, any> = Record<string, any>> = {
  target: Document | Element | ShadowRoot;
  anchor?: Node;
  events?: Record<string, (e: any) => any>;
  context?: Map<any, any>;
  intro?: boolean;
} & {} extends Props ? {
  props?: Props;
} : {
  props: Props;
}

Importance

nice to have

trueadm commented 2 days ago

Sounds like you're half way there towards making a PR :P

webJose commented 2 days ago

I would but I don't know JsDoc enough. I quickly gave it a shot. I defined the type in packages/svelte/types/index.d.ts, but the type needs importing in packages/svelte/src/internal/client/render.js. This is where I don't know: I see zero imports from the file I modified, and to my surprise there's an index.d.ts file in src. So I'm at a loss here. If you can guide me through this, I'll complete the PR.

brunnerh commented 2 days ago

Can you create a draft PR with what you already have?

brunnerh commented 2 days ago

Import option examples:

// These give you the `Snippet` to use elsewhere:
/** @import { Snippet } from 'svelte'; */
/** @typedef {import('svelte').Snippet} Snippet */

/**
 * Or inline import:
 * @param {import('svelte').Snippet} arg
 */
function fun(arg) { /* ... */ }
webJose commented 2 days ago

Here's the PR: https://github.com/sveltejs/svelte/pull/13674