Update to use svelte-kit (includes adding types)

[mhkeller]

This is a work in progress on the kit branch. It includes typescript support and will close #49

TODO

• Add types for typescript
• Figure out how to export fewer functions
• Excludes .DS_Store
• Get example working
• Add typescript types on context values (maybe slot values, too)?

[mhkeller]

I added a basic example to the repo itself and it seems to work. Not sure why it wasn't working from pasting it into another project's node_modules but i could also see that breaking for a whole host of reasons...

[mhkeller]

I added better export rules but still getting Package subpath './package.json' is not defined by "exports" inlayercake/node_modules/d3-scale/package.json` so i guess that was unrelated. Some things to read: d3/d3-scale#248 (comment)

[techniq]

@mhkeller Gosh that doesn't sound good, and I feel like I keep going deeper down a rabbit hole the more I follow.

[mhkeller]

I would think that the vite setting of optimizeDeps would bundle the d3-scale options but that doesn't seem to have any effect.

as a fallback tho, layercake isn't doing anything fancy – it's just a series of components, really. so you can download the source into your project and import it like you would any regular component.

[techniq]

In one of my private app repos I basically had all its npm packages referenced as it would help keep Vite from requiring multiple rescans on startup until it built its cache, and helped with SSR errors I was getting. After upgrading SvelteKit/Vite 2.7, I was getting TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".svelte" for ... errors until I commented them out. At the time, it appeared I could remove them all, but maybe I need to look into adding the d3/etc back. I'm not sure how transitive deps will be though (LayerCake => d3-scale, for example) as I don't think i can add LayerCake back...

Each major upgrade to SvelteKit/Vite always turns into hack a mole and then I'm afraid to touch things too much :(

[mhkeller]

yea i'm very confused by it.

[techniq]

Interestingly... adding d3-array, d3-format, and d3-time to optimizeDeps for my svelte-ux example fixes it...

vite: {
  optimizeDeps: {
    include: [
      'd3-array',
      'd3-format',
      'd3-time',
    ],
  },
},

and adding d3-scale, d3-shape, d3-time, d3-array, and d3-format fixes my LayerChart example

vite: {
  optimizeDeps: {
    include: [
      'd3-scale',
      'd3-shape',
      'd3-time',
      'd3-array',
      'd3-format'
    ],
  },
},

[mhkeller]

Try upgrading to "rollup-plugin-svelte": "^7.1.0" I just did that for the sapper site and it worked. You may have to update your rollup.config.js settings to add stuff under compilerOptions. I pushed the changes to this branch: https://github.com/mhkeller/layercake.graphics/tree/support-kit

[mhkeller]

Here's the diff: mhkeller/layercake.graphics#39

[mhkeller]

Okay I got all the props working with good pop-ups in VS Code. I'm trying to add them for the context values, now.

@abalmos and @pierreminik, do either of you know the proper way to document store values set on the context. Currently they just show up as any

[pierreminik]

For my custom stores I define an interface and initialise an initial object with that type on the writable() which seems to set the types on the values.

I have methods on the custom store (don't know if this is a good practise) but with typescript they only show up on store.method etc., while when I use $store.property the types for the data shows up.

[mhkeller]

Thanks. Do you have any example code?

[techniq]

@mhkeller I have a fair bit of stores using typescript in svelte-ux that might help. See for example fetchStore. These types can always be improved, but might be a good start.

[techniq]

@mhkeller Just realized you were asking about context... I think you have to set the type when getContext<...>('LayeCake') like for example here and here.

It might be helpful to add a wrapper around getContext<LayerCakeState>('LayerCake') to handle both the type and the context key. Maybe something like getLayerCakeContext() although something shorter would be nice :)

[mhkeller]

I'll have to dig into this a bit more. I had thought I could just do something where I write a context declaration like this:

// Do something where I import the Writable type definition from svelte

declare module 'layercake' {
  export interface LayerCakeContext {
    data?: Writable<array|object>, // <-- say here that this thing is a Writable store whose value is an array or an object
    xGet?: Writable<function> // <-- say here that this thing is a Writable store whose value is a function with signature array or object and returns any
  }
}

And then in each example component, it would include this line

/** @type {import('layercake').LayerCakeContext} */
const { data, xGet } = getContext('layercake');

My hangup is more on how do I write the typescript definitions such that typescript understands but references to data and $data. This declaration file seems to work but I got stuck on how to explain the properties of a Writable type.

If I create a wrapper function, that would be an API change so I'd like to avoid that – and wouldn't it mean writing these same store declarations just on a different function? So I don't totally see what it saves me but I clearly don't know that much about typescript declarations!

[mhkeller]

This kind of works:

/** @type {import('svelte').Writable} */

declare module 'layercake' {
  export interface LayerCakeContext {
    data?: Writable<array|object>,
    xGet?: Writable<{d: object}>({
      value: any
    })
  }
}

When I hover over xGet I see this

But hovering over $xGet is not helpful:

[mhkeller]

Turning away from this for a little bit but here's an update

This seems to work better

import { Writable } from 'svelte/store';

declare module 'layercake' {
	export interface LayerCakeContext {
		data?: Writable<array|object>,
		xGet?: Writable<{(d: object)}: any>
	}
}

Although I think there's a syntax error because any property declarations following that one are not recognized...
It would also be great to somehow add a description for each of these but I haven't come across how to do that yet...

[abalmos]

@mhkeller I have been defining a LayerCakeContext like this:

  export interface LayerCakeContext<Data> {
    data: Writable<Array<Data>>;
    x: Writable<(d: Data) => number>;
    y: Writable<(d: Data) => number>;
    y: Writable<(d: Data) => number>;
    r: Writable<(d: Data) => number>;
    zGet: Readable<(d: Data) => number>;
    rGet: Readable<(d: Data) => number>;
  }

without testing anything, a few comments on this:

import { Writable } from 'svelte/store';

declare module 'layercake' {
	export interface LayerCakeContext {
		data?: Writable<array|object>,
		xGet?: Writable<{(d: object)}: any>
	}
}

array -> Array<T>, where T is the datatype of what is in the array.
xGet?: Writable<{(d: object)}: any> -> xGet?: Writable<(d: object) => any>

Although I try hard not to use things like any, object, etc. because they don't offer much type checking. Considering factoring them out as generics so that the developer can decide what type is expected.

I am not familiar with the jsdoc notation, so I'm not sure if that is the correct why to type getContext. I suspect I would still need to something like:

const { data, xGet } = getContext<Data>('layercake')

in my TS project, where Data is the individual data type that layercake is managing in my app. I think this is fine, LayerCake can't possibly know that and getContext is just grabbing from an arbitrary object store. This is just a reality of getContext's design.

[mhkeller]

@abalmos Thanks that helps a bunch! This definitely gets me much farther and fixes my syntax error with the , instead of ;.

I'll work through the ones that are pretty easy to do and depending on how it goes, put in more specific types in later releases. And if you see any improvements, PRs welcome.

For example, I may set data to any for now. It can be an array but in the case of maps it can be a geojson object. Assigning it to array|object, though, is annoying if you do a call to $data.map unless you do Array.from($data).map

Similarly, the return value of xGet can be whatever the person sets into the domain so if it is often a number but can also be a string, or in stack layouts, an array of two numbers. I'd have to detect the type of what's in xDomain somehow, but not sure if that is too dynamic of a system.

A big value add would be to include text descriptions like shown here but they aren't showing up for some reason...

[techniq]

For those who don't have a strong type/interface for their data, you could also default the generic to any if not passed...

export interface LayerCakeContext<Data = any> {
    data: Writable<Array<Data>>;
    x: Writable<(d: Data) => number>;
    y: Writable<(d: Data) => number>;
    y: Writable<(d: Data) => number>;
    r: Writable<(d: Data) => number>;
    zGet: Readable<(d: Data) => number>;
    rGet: Readable<(d: Data) => number>;
  }

which would allow usage like this (within typescript)

const { data, xGet } = getContext('layercake')

for example this is how the spring store is defined, although ironically it is not set on the tweened store.

It's not terrible to require the user to pass any in these situations as well though...

const { data, xGet } = getContext<any>('layercake')

so it comes down to how much you want the user to stop and think about it. I'm indifferent either way, although I do traditionally add it to my stores, but also try to have T inferred from the arguments passed when possible, for example here.

[mhkeller]

I've tried to do it through jsdoc but it doesn't seem to work: https://stackblitz.com/edit/sveltekit-vbaqsd?file=src%2Froutes%2Findex.svelte (not sure if stackblitz supports the hover type hints but I put it up here just to show the setup)

Creating the interface did work to get the props documented but I'm less concerned about type hints since for things like domains and scales, it will often change. What would be helpful, though, are text comments, hence trying to do it through jsdoc. I couldn't find a way to put text comments into the interface definition. I may put this on hold for now unless anyone has a solution to get comments coming through. Thanks for all the help so far!

[mhkeller]

@dummdidumm since you were really knowledgeable on #58, I'm wondering if you have any thoughts on any good way to pull comments on a context object. This is the last thing on my list before I push up the svelte-kit version of this lib so I'm trying to sort out if this is feasible at the moment or I should let it go / await some future feature that will allow this.

[mhkeller]

Looking at the svelte/store .ts files, it seems like I should be able to add comments like this inside my global.d.ts file

mport { Writable, Readable } from 'svelte/store';

declare module 'layercake' {
  /** Configured and computed values */
  export interface LayerCakeContext {
    /** information here */
    data?: Writable<Array<T>>;

    /** More information */
    x?: Writable<(d: Data) => number>;
  }
}

And this does give me hovers when I'm inside that file:

From within a component if I do this, though, i only get the type definition

/** @type {import('layercake').LayerCakeContext} */
const { data, xGet, x, y, yGet, xScale, yScale, extents } = getContext('LayerCake');

I wonder if those comments just aren't supported yet.

[mhkeller]

Adding one more experiment here, which makes me think it's not supported. I've added these jsdoc comments into my index.js file

/**
 * Configured and computed properties to use
 * @typedef {import('svelte/types/runtime/store').Writable} Writable
 * @typedef {import('svelte/types/runtime/store').Readable} Readable
 * @typedef {Object} LayerCakeContext
 * @property {Readable<Function>} x x whatever
 * @property {Function} y a thing called y
 */

And the hovers show the type definitions but not the comments:

This approach is actually nicer than the global.d.ts approach because I get a full list of properties when hovering over the definition

I am getting a weird error though that Writable is not generic?

[dummdidumm]

You could do something like this:

interface LayerCakeContext {
    /**
     * The date you passed.
     */
    data: any;
    /**
     * X coordinate
     */
    x: number;
}

function getLayerCakeContext(): LayerCakeContext;
function getLayerCakeContext<Key extends keyof LayerCakeContext>(key: Key): LayerCakeContext[Key];
function getLayerCakeContext<Key extends keyof LayerCakeContext>(key?: Key): any {
    const context = getContext('LayerCake');
    return key ? context[key] : context;
}

This would give you a way to call getLayerCakeContext to get the whole context and getLayerCakeContext('data') to only get the part from it you are interested in. It would also provide you the correct types.

Unfortunately there's no way to make the comments appear this way, too, when you destructure the result. This is not a Svelte problem, it's a TS problem, these are the related issues for that:

• JSDoc support for destructured parameters microsoft/TypeScript#11859
• Copy JSDoc from type when destructing arguments. microsoft/TypeScript#27601

So this means your way of using a JSDoc comment on the caller side is the only way if you want to have the comments, too.

[mhkeller]

Thanks a bunch @dummdidumm! Got it. So I think I'll skip this for now, then. I may add some basic types but things like scales, it will likely just be a lot of any types and not super useful.

[mhkeller]

This is now merged and released as 6.0.0 https://github.com/mhkeller/layercake/releases/tag/v6.0.0

[dit7ya]

I am a bit confused as I cannot find any examples of TypeScript types for LayerCake. Could you please tell me what is the recommended way for it?

[mhkeller]

You should be getting type ahead documentation for props but that was pretty much all that made sense. I don’t think this library lends itself that much to strict types since your scales can change the signature of your domain and ranges. In any event, Layer cake doesn’t really care since it’s more of a pass-through. If you’re running into errors feel free to open an issue.