Evercoder / culori

A comprehensive color library for JavaScript.
https://culorijs.org
MIT License
817 stars 30 forks source link

[Feature Request] TypeScript Definitions #128

Open Enteleform opened 2 years ago

Enteleform commented 2 years ago

Just following up on this comment by @danburzo

Just to make sure you're not doing too much work upfront: if the changes are related to TypeScript, please be aware that culori is intentionally a JavaScript, not a TypeScript, project. (One TypeScripty thing we can do in this repository is maintain a t.ds typedef file, if that is helpful.)

Yes please! That would be very helpful and make the library much easier to use via IntelliSense.

akx commented 2 years ago

As mentioned around that comment, there's a partial .d.ts in https://github.com/akx/gradient/blob/master/src/culori.d.ts that could work as a starting point.

akx commented 2 years ago

By the way - something I noticed e.g. https://github.com/syntax-tree/mdast-util-from-markdown does is use TypeScript's JSDoc support to allow type annotations, inference and declaration file creation in plain-JS files: https://github.com/syntax-tree/mdast-util-from-markdown/commit/052ad823f055d04b0e5325366b970162a743bd4e.

Would you be amenable to something like that, @danburzo? I could look into how that might work here :)

ai commented 2 years ago

JSDoc syntax looks like very limited. For instance, culori may need more complex types for color object:

export type Color = string
  | { mode: 'rgb', r: number, g: number; b: number }
  | { mode: 'lch', l: number, c: number, h: number }

I am not sure that it is easy to define such types in JSDoc. At least it will not be very readable and maintainable.

WebMechanic commented 1 year ago

It's tedious but possible to do this with JSDoc and type definitions can go into separate files. Stuff like this in some external typedefs.js within the package/project should provide decent IntelliSense i.e. in VSCode or WebStorm. You can't refer to or enumerate keys for use in other @typedefs. Also iirc @extend only applies to Classes not Types.

/**
 * @typedef {Object}  RgbMode
 * @property {String} mode  'rgb' Color mode name
 * @property {Number} r  Red component
 * @property {Number} g  Green component
 * @property {Number} b  Blue component
 * /

/**
 * @typedef {Object}  LchMode
 * @property {String} mode  'lch' Color mode name
 * @property {Number} l  Luminance component
 * @property {Number} c  Chroma component
 * @property {Number} h  Hue component
 * /

/**
 * @type {String | RgbMode | LchMode } Color
 */

/**
 * @param String color
 * @return Color
 */
const parseHwb = color => { ... }

That's just two colour mode types hacked together (can't recall how to declare default values) and it'll be very, very elaborate to annotate each function inline unlike declaring types and interfaces in a separate .d.ts which the IDE would use as well. After all it's very likely much easier and shorter to create a proper .d.ts.

bijela-gora commented 1 year ago

@WebMechanic hi!

The PR that adds TS types is ready https://github.com/Evercoder/culori/pull/166

Do you think it is possible to do the same with JSDoc? Does it make sense?

WebMechanic commented 1 year ago

Hello @bijela-gora, I wasn't aware of the PR. Well done!

The examples I was giving are very basic. There is much more, but JSDoc can't replace the granular details of (real) type definitions and unlike them it doesn't infer information from existing code. It "knows" what you add on top inside the code comments and the IDE will present what you write. It's purpose is to add supplemental static documentation about what a class, function, property or parameter does. You enrich plain comments with external links and additional cross references where the source code itself doesn't tell you about possible relations.

Most IDEs and code editors can leverage JSDoc contents to give similar feedback like type definitions, such as squiggly lines if you mistype a param or pass the wrong data type to a function. The TSC certainly doesn't give a toss about them and won't validate anything. By adding JSDocs you do get to see comments from other parts of the project in these IntelliSense popups or code lenses. With JSDoc you basically pimp up a simple text comment and make it interactive. Editors provide clickable references between the various parts of the code.

So, can it do the same? No, not on the type and data level detail, but one can add other useful and supporting things to these sources. JSDoc are less about validating the code one writes, but about understanding it as a user.

Does it make sense? As a supplemental to what you added, for sure. I'd argue as a user of any 3rd party code it's always nice to have some additional documentation right in your editor at your fingertips beyond bland data types. I don't think many library users read the tests to understand what some code will do, but they'll read the documentation the IDE picks from the JSDoc and puts it right in front of them. I'm not a library author and I can barely remember what any code from a 5 or 7 years old random project of mine does, let alone other people's code I "just want to use".

Thanks for your work!

bijela-gora commented 1 year ago

@WebMechanic

Despite I have added descriptions to functions, classes, methods before when it seemed important, аfter your comment, I feel that I will be more attentive to the documentation for developers, and will take this use case into account.

Thank you!

WebMechanic commented 1 year ago

FYI you can always sprinkle some JSDoc fairy dust onto your .d.ts as you see fit.

/**
 * @param [amt]  Amount of filter applied
 * @param [mode] The mode parameter is expected to be 'rgb' or 'lrgb', because filters were designed for RGB colors.
 */
type Filter = (amt?: number, mode?: Mode) => ColorToColor;

In this case the parameter data types are taken from the declaration (no need to repeat them then) and their decription from the JSDoc.

image

bijela-gora commented 1 year ago

@ai @Enteleform @akx @WebMechanic

https://www.npmjs.com/package/@types/culori

ai commented 1 year ago

@bijela-gora works great! Thanks.

https://github.com/evilmartians/oklch-picker/commit/5efd07bd3d0d56622c6c4c1141dd3a6fd8a45a76

swernerx commented 4 months ago

Any updates on the issue?

https://www.npmjs.com/package/@types/culori is fine, but misses a few API methods e.g. toGamut.

drwpow commented 3 months ago

Would the maintainers now be open to a PR that updates the library to be written in TypeScript (in case opinions may have changed in the 3 years since this issue was opened)? That way it ships its own definitions. I’d be happy to open a PR if desired 🙂

We could also update the test suite to use Vitest, which is not only spec-compliant and supports TypeScript, it’s fast, too (only in case test runner is a potential blocker to migrating to TypeScript)

danburzo commented 3 months ago

Hi @drwpow, my thoughts on the matter can be found here, and it’s still the case that migrating to TS will introduce a maintenance burden that I cannot sustain at the moment, in addition to aesthetic concerns for the library (the intentional removing of as much tooling / dependencies as possible). The previous PR made apparent just how much new API design surface a migration entails (naming things, etc.).

If incremental adoption of a JSDoc-to-types sort of thing were feasible, that would be a good compromise I think.

drwpow commented 3 months ago

@danburzo thanks for replying! And thanks for re-posting that more recent response; I hadn’t seen that PR/discussion/context.

That PR is actually superior than the JSDoc approach, because when shipping a library to consumers you really need those .d.ts files. TypeScript can attempt to generate those from JSDocs, but there are far fewer features available. If Culori was difficult to type in .d.ts files, it would likely be impossible with the JSDoc approach.

That’s why .ts exists, basically—to not have to manage JSDoc types, or .d.ts files, or anything else, really—you just throw JS in a .ts file and everything Just Works™. In fact, the best TS uses the absolute fewest types possible, and leans on inference.

This issue also is important because the DefinitelyTyped solution of @types/culori is a temporary workaround, and there are already errors in how that’s typed. For Culori to continue to be used, it really should ship its own types that can be packaged with the library; otherwise this can’t be used in any meaningful production capacity (and Culori is so fast, so accurate, so well-designed, and hands-down the best color library in JS; it would be an absolute shame if people had to use “worse” color libraries just for the omission of TS types).

All that said, I completely respect your decision, and it’s important to set healthy boundaries on open-source projects. I just selfishly want to beg you to reconsider 😄. As someone that’s used TS for years, I would strongly disagree it’s a maintenance burden—the opposite, in fact. It saves you from having to write so many tests, and the tooling is so mature now it’s trivial to compile. It is a learning curve, but hopefully a short one with the right guidance. And I (and I’m sure others) can help lighten the load so that the migration isn’t scary, or “too much,” and the end result still looks, feels, and operates like the exact-same code.

danburzo commented 3 months ago

Thank you for that perspective, and for the kind words.