kpdecker / jsdiff

A javascript text differencing implementation.
BSD 3-Clause "New" or "Revised" License
7.69k stars 491 forks source link

Adding Types #496

Open leonardobsjr opened 4 months ago

leonardobsjr commented 4 months ago

Adding TS types.

ExplodingCabbage commented 4 months ago

Hmm. So I see that this adds declarations of the functions that jsdiff exports but all parameters and return types are just set to any, which makes them kinda non-useful. I guess I could use those as a start point and manually modify them. But it'd be nice to have useful types generated automatically.

I'm not too sure what the best way to do that is, though. Probably I should port the whole library to TypeScript, adding types to function signatures as I go, and change its build process to compile from TypeScript?

ehoogeveen-medweb commented 4 months ago

Another way would be to add jsdoc comments for the exported functions - IIRC those get picked up by tsc and used in the generated declaration files.

ExplodingCabbage commented 4 months ago

Ah, nice! That does indeed seem to work and I guess it'll do the trick.

leonardobsjr commented 4 months ago

Yeah, I understand that this is not ideal @ExplodingCabbage - This is just a way of making sure that every jsdiff version has up-to-date method information to be used in typescript. The issue that motivated me was not being able to use a method that existed on the latest version of jsdiff because the type was not updated on https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/diff.

That being said, @ehoogeveen-medweb suggestion is far better than porting the whole stuff lol

ehoogeveen-medweb commented 4 months ago

Well, I don't know about better as I do like TypeScript a lot, but it's certainly a lot less work ;)

ExplodingCabbage commented 3 months ago

Okay, coming back to this after a break...

I think porting to TypeScript is probably actually no more work than adding JSDoc comments would be. One problem with adding JSDoc comments is that some TypeScript concepts don't quite have clean analogues in JSDoc. For instance, the diffChars / diffWords / etc functions have a bunch of common options, so it makes sense to define a BaseDiffOptions interface and then have DiffCharsOptions extends BaseDiffOptions, DiffWordsOptions extends BaseDiffOptions, etc, but JSDoc doesn't seem to cleanly support this. Defining a shared type that's used in multiple places and then importing it from other files seems to be a bit weird/tricky/verbose in JSDoc?

I'm gonna have a stab at actually porting everything to TypeScript and using Babel's preset-typescript to transpile to JavaScript (as seems to be the recommendation at https://www.typescriptlang.org/docs/handbook/babel-with-typescript.html if you need a build pipeline anyway); I'm hoping that besides actually sprinkling TypeScript types into all the function definitions, there's not gonna be much work involved in this beyond adding a few lines to the Babel config. Will see if that hope is proved accurate...

ehoogeveen-medweb commented 3 months ago

By the way, one thing that might help in this process is https://arethetypeswrong.github.io or https://www.npmjs.com/package/@arethetypeswrong/cli (right now there are no types, but once there are, getting CJS and ESM to both work can be tricky).