Inline @types/diff

[isaachinman]

Resolves #303

[ExplodingCabbage]

We don't need the .mts file from @types/diff? How come it's needed there but not here?

[ExplodingCabbage]

Also - I note that the @types/diff version had comments in it, which I assume would sometimes get rendered (e.g. in a user's editor when hovering over an invocation of one of the functions declared in the .ts file), but you've stripped them out here; is there a reason for that? I would've thought we'd want to include such comments, assuming they had some purpose to begin with (though perhaps they are way out of date by now and in need of updating).

[isaachinman]

Yeah good catch on the comments.

As far as .ts vs .mts, I assume @andrewbranch did this to handle an ESM entrypoint:

DefinitelyTyped/DefinitelyTyped#69537

I believe a top-level types in package.json should cover both cases, and you're already managing multiple entrypoints in your Gruntfile.

Happy to amend as requested/needed. Feel free to push yourself if that is quicker. This is a good starting point at least!

[ExplodingCabbage]

👍 Cool, thank you. I'll have a play around when I have a chance (probably Monday), trying to consume the types in as many different ways as I can think of and compare the experience between the old @types ones and this. If they seem to behave at least as well in every way, or I can make them do so, I'll merge.

[andrewbranch]

This doesn’t look right to me—TLDR a .mjs file needs a .d.mts file, no exceptions. As the PR is now, you’re going to hit this problem: https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseCJS.md.

What’s the motivation for doing this? It makes sense for libraries that generate their types to ship them, but DefinitelyTyped has tooling, processes, and community experts to help validate that hand-written types are correct. As far as I can tell, moving from DT to here is purely a loss.

[ExplodingCabbage]

What’s the motivation for doing this?

Speaking for myself (@isaachinman's thinking may not be identical):

1. It's good for people not to have to install @types packages; that's an extra obstacle to getting started using the library
2. I'd like the release of new library versions & corresponding types to be atomic and to eliminate the possibility of the types version being out of sync with the implementation version.
3. There are some issues or possible issues with the types file in DefinitelyTyped that I'd like to fix (e.g. at least some comments are out of date and now contain incorrect information, like the one saying maxEditLength won't work for patch-creation functions besides structuredPatch; some properties aren't listed there like the stripTrailingCr option to createTwoFilesPatch; we've got an open GitHub issue on this repo complaining that something is broken with the typing of the intlSegmenter option). I figured the best way to go about fixing these was simply to adopt the types and fix them here.

In my mind the benefits of 1 and 2 seem worthwhile even if the types remain hand-written (and of course the same people who were maintaining them over at DefinitelyTyped would be wholly welcome to keep contributing type-related PRs and commentary here). You disagree?

I can also read over the types and chuck a big PR at DefinitelyTyped fixing them in their current home, and I figure I might as well do that as an uncontroversial first step - but tbh I'm surprised you wouldn't want us to take ownership of them! I'd like to understand that perspective difference.

(Honestly, I was thinking that once I'd figured this out it might also make sense to propose an idiot's guide, to include in the DefinitelyTyped README, on how a package maintainer should go about adopting types and tests from DefinitelyTyped into a project while keeping them hand-written - but it sounds like that's not even something you ever want people to do!)

[isaachinman]

My opinion: DT is a relic and all actively-maintained OSS should move away from it.

This PR could easily be fixed up to address @andrewbranch's concerns about CJS/ESM, but I don't have the time or energy to get involved in politics.

@ExplodingCabbage Having heard your intentions, I would strongly suggest just rewriting the entire repo in TS. It's not a very large codebase and would be quick to do. You'd then be shipping your own types in the "right" way.

[ExplodingCabbage]

I will wait a few days before doing anything and let @andrewbranch weigh in, should he choose to.

Having heard your intentions, I would strongly suggest just rewriting the entire repo in TS.

Yeah I've considered this and may yet do it at some point. But it would require ripping out the old build system (from before I was maintaining this library) that I don't understand very well and uses no-longer-popular tools like Grunt, and replacing it with modern tools, ideally without breaking compatibility at all. I can probably pull that off with care but the idea makes me nervous due to me not having a very detailed understanding of the different module systems that exist in the JavaScript ecosystem or how to comprehensively test that I haven't broken compatibility with any of the ways of importing stuff that are currently supported; it would be very easy for me to break compatibility with some environments and not have a clue I'd done so. It's probably worth me studying and figuring out how to do this stuff, but I figure it's probably something I need to find the time to dedicate a whole weekend to.

Adopting the handwritten types feels easier than the above and is probably fine given I don't foresee doing many more releases, ever, at this point; I've fixed most of what I initially wanted to fix when I volunteered to maintain this library.

[isaachinman]

the idea makes me nervous [...] it would be very easy for me to break compatibility with some environments

This is what semver is for. I'd advocate going for it, do your best, release on a new major, and work through any issues that are raised.

[andrewbranch]

Full disclosure: I work on TypeScript for Microsoft, help steward DT, and wrote some of the tooling on DT that helps detect potential problems.

My point is just this: it sounds like @ExplodingCabbage was prepared to do the due diligence on this PR to try it in a lot of different configurations, so hopefully the CJS/ESM issue would have been caught. But this stuff is complicated and not very widely understood, and it would have been easy to miss the combination of imports and tsconfig settings and package.json fields that would have shown errors. I’ve seen many popular libraries ship subtle module-related typings mistakes that break tons of their users, so it’s not too hard to imagine it happening here. But if someone had removed the .d.mts file on DefinitelyTyped, automated tooling would have caught the mistake and CI failed. And because @types/diff is so widely depended upon, the PR would have gotten whatever reviews and help it needed to get fixed up. If you want to move away from DT, I think it’s worth putting some of those safeguards in place for yourself. Some ideas for places to start:

• Use tsc to type check the .d.ts files you commit; even better, type check a .mts file and a .cts file importing your library; use a range of different tsconfig settings.
• Use @arethetypeswrong/cli (written by me) to automate that and do some additional checks
• Use tsd to test your types (you could port tests from DT)

My opinion: DT is a relic and all actively-maintained OSS should move away from it.

We’re generally excited to see projects move away from DT (less work for us!), but that usually means they’re being written in TypeScript. But DT is never going away; ask @types/react or @types/node 😄