leeoniya / uPlot

📈 A small, fast chart for time series, lines, areas, ohlc & bars
MIT License
8.51k stars 371 forks source link

typescript type defs #147

Closed TiredFalcon closed 4 years ago

TiredFalcon commented 4 years ago

This seems to be a pretty efficient and amazing library.

Unfortunately I have been unable to successfully use it in my application with Vue.js using Typescript. The library lacks type declarations, and in general lacks the ability to be imported (e.g. with an ES6 import, import uPlot from 'uplot';) and used programmatically to render a chart.

Are there plans to create type declarations for the library? Without them, the library is more or less unusable in any modern JS framework.

photonstorm commented 4 years ago

I'm using it in a TypeScript project right now, using Rollup to bundle it. I import it exactly as you've commented above and it works fine.

Yes, it is missing TS definitions, but that doesn't stop it being used. It just means you have to do things the old-way and consult the examples / source / docs, rather than rely on code-insight.

leeoniya commented 4 years ago

Are there plans to create type declarations for the library?

if someone wanted to contrib a type defs file, i'd gladly answer any pertinent api questions, though i would not be qualified to review it extensively, since i'm not typically in TS. this is especially true if the types are made extra terse/DRY through generics and multiple layers of indirection.

TiredFalcon commented 4 years ago

Alright, now importing with import uPlot form 'uplot'; and adding a declarations file (e.g., src/shared/types/uplot.d.ts, containing declare module 'uplot';) solves the problem. I think this could be added to the documentation, with a short note about the need to declare the module if using Typescript.

For now, I guess this is an ok solution, but I do believe typings would be very useful. I am not well versed enough in Typescript to create declarations, although if I manage I'll gladly submit a PR.

photonstorm commented 4 years ago

Strange, I didn't have to create a module at all. VSCode happily let me carry on with an implicit any type and it compiled fine.

From what I've seen of the codebase so far, creating TS defs will be quite tricky. The function calls are easy enough, but the configuration options are vast. Probably worth waiting for the API / configs to settle down a bit before starting this imho.

leeoniya commented 4 years ago

From what I've seen of the codebase so far, creating TS defs will be quite tricky. The function calls are easy enough, but the configuration options are vast.

indeed, this will not be a quick 1-hour job, but not terribly daunting either. if my TS-foo was better, i'd do it myself, but there are only so many unpaid hours in a day i want to dedicate to side projects.

Probably worth waiting for the API / configs to settle down a bit before starting this imho.

1.0.0 was tagged last night, so the API & opts are pretty much settled. there are some extra callback args in places that i'd prefer to under-document so i can break them without upsetting semver, but those are a tiny minority.

a good place to start is to browse #48 and inspect a uPlot instance - it re-exposes the majority of its opts (after merging default opts with user opts) and should be a pretty thorough scaffold for typedefs.

TiredFalcon commented 4 years ago

Strange, I didn't have to create a module at all. VSCode happily let me carry on with an implicit any type and it compiled fine.

I am using WebStorm and I have an ESlint configuration that disallows implicit any types. I'm not sure that's the same issue though, the problem was really that it could not find type declarations for the imported module, so I had to declare it somewhere.

danyalejandro commented 4 years ago

If this is not being written already, I can give this a shot over the weekend, been using TypeScript daily

leeoniya commented 4 years ago

yeah, any help would be appreciated.

i've been trying to find a simple template on https://github.com/DefinitelyTyped/DefinitelyTyped to start with, but they are all so different based on how libs export stuff that i could not isolate much in common. e.g. some have namespaces, others don't. some have multiple files and are very verbose with comments, others have some default file that exports just the lib name 🤷‍♂

fluxin commented 4 years ago

I added some initial types in a PR I created

153

leeoniya commented 4 years ago

thanks @fluxin!

i'm currently on vacation, but i'll review your PR in the next few days.

leolabs commented 4 years ago

I modified and added the types of @fluxin to DefinitelyTyped, for now, to use uPlot in a TS project: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43250. Feel free to remove the definitions again when your repo provides their own.

leeoniya commented 4 years ago

hey @leolabs, i would prefer that you close that PR.

there are a good amount of inaccuracies and many omissions in #153, which i'm working through right now.

i expect the proper types to land in a day or two.

leolabs commented 4 years ago

Hey @leeoniya, I'm sorry for the inconvenience. I closed the PR and would gladly like to help with implementing types for uPlot. Maybe the changes I made will be helpful to you :)

fluxin commented 4 years ago

Sorry for not being more clear on the initial commit. I've never made a definition file before and only spent about an hour trying to figure it out. So I would not consider it complete or fully correct. Just useable based on features I personally used in a project. I just hope it would be a good baseline for someone to start with.

On Thu, Mar 19, 2020, 6:28 PM Leon Sorokin notifications@github.com wrote:

hey @leolabs https://github.com/leolabs, i would prefer that you close that PR.

there are a good amount of inaccuracies many omissions in #153 https://github.com/leeoniya/uPlot/pull/153, which i'm working through right now https://github.com/leeoniya/uPlot/pull/153#issuecomment-601383507.

i expect the proper types to land in a day or two.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/leeoniya/uPlot/issues/147#issuecomment-601462409, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAWBLETFVB6QJCRYMOBQTRIKTAFANCNFSM4LFDQJWA .

leeoniya commented 4 years ago

alright, folks. here it is!

https://github.com/leeoniya/uPlot/blob/master/dist/uPlot.esm.d.ts

improvements and/or corrections are welcome.