DefinitelyTyped repo overhaul - docs/automatisation/formatting

[Hotell]

This isn't an issue related to any of type definitions rather to improve this repo and make it more consistent and more user friendly for old/new contributors.

Repo updates

• we really need consistent formatting ( incorporating prettier is viable choice )
• provide npm scripts for executing prettier + linter fixes so user doesn't have to do it on it's own manually
• provide githooks ( pre-commit 👉 formatting and lint fixes applied automatically )
• provide automated changelog generation with every release ( this is a must have as breaking changes can occur even on fix or feat semver bump, and it's not very user friendly to go through commit log what changed in some particular package )
• be able to run whole test suite from local machine and not wait for CI
• provide some codemods/scripts for updating all packages dependant on some other core package
  
  
  
  • for example: I update react types with some breaking change from type safety perspective ( bump minimum TS version ) or tweak API type coverage, now I have to manually go through 5k+ packages and update all of them ( which is super annoying especially if you have to resolve merge conflicts )
  
  
• enforcing consistent commit messages

Docs updates

• consistent pull request/issues naming
  
  
  
  • as can be observed a most used pattern is [package-name]: issue/pr description
  
  
• make clear what a breaking change means
  
  
  
  
  • IMHO it should be a breaking change only if the library bumps semver as breaking ( if we change some types to be better and expose new type-errors, it should not be bumped == same methodology as TypeScript team is doing )
  
  

Summary

If we agree at least at some of these points, I can start work on this immediately 💪

cc @sandersn @andy-ms @mhegazy @DanielRosenwasser

[sandersn]

Tagging @RyanCavanaugh, who is also interested in DefinitelyTyped hygiene.

[sandersn]

Also

be able to run whole test suite from local machine and not wait for CI

This is a documentation problem; we already can do this, I just can't remember how without searching around.

[RyanCavanaugh]

we really need consistent formatting ( incorporating prettier is viable choice )

Agree!

provide npm scripts for executing prettier + linter fixes so user doesn't have to do it on it's own manually
provide githooks ( pre-commit 👉 formatting and lint fixes applied automatically )

I think this would be great.

provide automated changelog generation with every release ( this is a must have as breaking changes can occur even on fix or feat semver bump, and it's not very user friendly to go through commit log what changed in some particular package )

What is your definition of "release"?

be able to run whole test suite from local machine and not wait for CI

As Nathan said, it is possible, probably just needs better documentation. Be warned that it takes a very, very long time

provide some codemods/scripts for updating all packages dependant on some other core package

I don't know what this means as a general functionality, but am on board in principle.

enforcing consistent commit messages

In general we already have a pretty heavyweight process. I don't want to enforce new rules that people have to follow and we have to enforce unless there's a very clear upside. We can already mine which PRs affect which packages and generate a changelog based on the PR titles (which are much easier to edit than commit messages).

consistent pull request/issues naming
as can be observed a most used pattern is [package-name]: issue/pr description

Again I'm not sure what the upside is here, or what the naming convention would be for changes which affect multiple packages

make clear what a breaking change means

Any non-trivia edit is a potential breaking change. We try to recommend that people pin their types dependencies and upgrade intentionally instead of floating for this reason. We don't vary the major version number anyway, so trying to draw an arbitrary line for what's breaking and what's not doesn't seem to give us much.

[binki]

@RyanCavanaugh

make clear what a breaking change means

Any non-trivia edit is a potential breaking change. We try to recommend that people pin their types dependencies and upgrade intentionally instead of floating for this reason. We don't vary the major version number anyway, so trying to draw an arbitrary line for what's breaking and what's not doesn't seem to give us much.

I think changes which would introduce new type errors for people should be considered breaking. When type checks fail, I consider my own build broken. If the changes to the type definitions are not backwards compatible and I’m consuming the definitions using semver, my build may break without any action on my part.

So, why not just always bump the major version for each release (where a release is a publish of the package to npmjs)? Types are very brittle and can easily break projects using them unintentionally or even for minor improvements, so it is likely quite difficult to determine what breaks definitions. Bumping major on every release for definition changes would let people install the typings with npm -D and be all set instead of needing to do something special when installing typings to get the proper versioning in their projects.

[RyanCavanaugh]

So, why not just always bump the major version for each release

This is just a different way of not having a coherent versioning scheme, with the disadvantage that you lose all mapping from the library version to the typing version. What's the latest typing library for foolib v5? Today it's the highest @types/foolib@5.*, and we can maintain separate version chains in a logical way. In the "semver means all is break" world, it's... a query against a remote database.

[binki]

Ah. So why is that line in the README, then? Because of it, I assumed there shouldn't be such a correlation between version numbers in the first place.

[thw0rted]

This issue is the first time I've seen anybody mention not running tests locally. It prompted me to search the front page, and I was surprised not to see it called there explicitly. npm run test works for me... for a very limited definition of "works". Over the past week, it seems like every day something new is broken, either due to changes in dtslint or typescript@next or some combination of stale deps installed in my node_modules. (Latest issue is a boatload of spurious strict-export-declare-modifiers and void-return lint violations.) I'd love to see run test called out explicitly in the contributor docs, but I'd also love to see it behave consistently on my machine from day to day.

I had thought I must be doing something wrong, since I didn't see a lot of people here filing issues about broken tests, but maybe the truth is that most people don't run them, so if something is brittle it doesn't get noticed as quickly as it otherwise might.

[Jessidhia]

I'd like to bump this thread for the sole reason that I have to remember to disable the prettier plugin almost every time I am working on anything in DefinitelyTyped 🤣

Adding a new lint step right now probably won't work well because CI is close to a breaking point (any given change to @types/react may randomly fail depending on how much CPU time the travis VM gets, for example); but if it's just "adding a command you run" to the top level package.json, well... I could do this now, even.

Adding a canonical config to the root would also be very helpful, even if it's an empty config (i.e. prettier defaults), because otherwise customizations done in the vscode settings take precedence. Personally I have semicolons off and single quotes everywhere, for example.

[Jessidhia]

I did it on the entire repository just to see what happens.

https://github.com/DefinitelyTyped/DefinitelyTyped/compare/master...Jessidhia:prettier

https://github.com/Jessidhia/DefinitelyTyped/compare/prettier...Jessidhia:prettier-lets-see-what-happens

25401 files changed, 1959969 insertions(+), 1425876 deletions(-)

[ExE-Boss]

I’m doing this piecemeal, starting with #34894 (and #34892)

[damassi]

This just happened to me as well -- I ended up running a commit with local prettier enabled and it introduced an unneeded diff. Would be great if there was some consistency here, including pre-commit hooks that apply lint --fix and formatting.

[alloy]

As a maintainer, I would _love_ to see a standardised prettier config, githooks enforce formatting, and perhaps prettier checking on CI. I have been running prettier on some of the types I maintain manually with the following configuration, which I sourced from looking at what some randomly picked DT types were doing:

$ prettier --parser typescript --tab-width 4 --semi --trailing-comma es5 --write --print-width 120

[alloy]

Another stab at adding prettier and git pre-commit hook is available at https://github.com/DefinitelyTyped/DefinitelyTyped/pull/35672

[damassi]

@RyanCavanaugh - wondering if you have any thoughts on https://github.com/DefinitelyTyped/DefinitelyTyped/pull/35672?

[rohit-gohri]

The command npm run prettier mentioned in Readme(here) doesn't work.

The output is :npm ERR! missing script: prettier

The command should be npx prettier path/to/package/** --write I think?

[alloy]

@rohit-gohri prettier is listed as a dev dependency, so after doing a npm install it _should_ work.

[rohit-gohri]

@rohit-gohri prettier is listed as a dev dependency, so after doing a npm install it _should_ work.

But there is no corresponding script. AFAIK only yarn runs package bins directly(yarn prettier works), and npm has npx to run local/global packages or temporary install and run them.

[alloy]

Ah I see, yeah that might have been based on yarn indeed. Care to send a PR to add the script?

[montogeek]

It is there any plan to create a changelog/website/docs something?