commitlint

commitlint copied to clipboard

Expected Behavior

ts-node should be an optional dependency of the project

Current Behavior

currently, installing @commitlint/cli results in a lot of downloaded NPM packages.

Affected packages

• [x] cli
• [ ] core
• [ ] prompt
• [ ] config-angular

Possible Solution

Instead of statically importing cosmiconfig-typescript-loader, @commitlint/load could depend on on it using an optional peer dependency and dynamically require / import() that dependency if/when a .ts file is detected, and show a warning if that fails:

Using TypeScript @commitlint configurations requires `@commitlint/ts` to be installed as a peer dependency of your project

Steps to Reproduce (for bugs)

Not really a bug.

$ yarn install @commitlint/cli

Context

I don't use Typescript. So I really don't see why ts-node should be a mandatory dependency of my project:

$ y why ts-node

yarn why v1.22.18
[1/4] 🤔  Why do we have the module "ts-node"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "_project_#@commitlint#cli#@commitlint#load#cosmiconfig-typescript-loader" depends on it
   - Hoisted from "_project_#@commitlint#cli#@commitlint#load#cosmiconfig-typescript-loader#ts-node"
info Disk size without dependencies: "1.5MB"
info Disk size with unique dependencies: "2.97MB"
info Disk size with transitive dependencies: "3.25MB"
info Number of shared dependencies: 16

Your Environment

Executable	Version
commitlint --version	@commitlint/[email protected]
git --version	git version 2.36.1
node --version	v16.15.0

Relates to #3218 I guess.
Happy for a PR if you're motivated and have time.

Had a look where this was introduced: https://github.com/conventional-changelog/commitlint/issues/3007#issuecomment-1030568481

So looks like we wanted to avoid a breaking change. Happy for a PR which is cleaning this up and create a major version. Maybe @jrolfs has some feedback to this as well.

This absolutely makes sense to me. I think it might be a little tricky to implement, though, because isn't the point of Cosmic Config to delegate configuration discovery (which files etc.)? I guess, even as someone who uses TypeScript almost exclusively, I'd also question the value of supporting TypeScript-based configuration files. I think the easiest move here might be to drop support for those, but I don't know how popular that feature is.

Hrm, maybe we could wrap the loader in a function that ensures the optional dependency:

import { cosmiconfig } from "cosmiconfig";
import TypeScriptLoader from "cosmiconfig-typescript-loader";

const moduleName = "module";
const explorer = cosmiconfig("test", {
  searchPlaces: [
	// ...
  ],
  loaders: {
    ".ts": TypeScriptLoader(), // ← wrap this in a function that loads `cosmiconfig-typescript-loader` dynamically
  },
});

const cfg = explorer.load("./");

Thanks for your feedback @jrolfs !
There were quite some people involved to add the TS support. I believe we should support this because this is were projects are heading in the future.

Not sure how many people use a TS config. But also not sure how many people care about the size of commitlint really (Relates to #3040, #1791). I'll let this rest a bit and might get back to it at a later point.

But also not sure how many people care about the size

I just wanted to advocate for the people who care about the size since we're heavy users of your libs (btw thx for it 🙏).

The hard-dep makes our docker image nearly ~100MB bigger, which may seem not much nowadays, but equates to more bandwidth usage which is costly, and cumulative time wasted by developers waiting image pulls in CI/locally. This is just a matter of DevX to me :)

Reopening because of #3641 This will then hopefully be fixed with a new mayor version. yolo

~~I'm also seeing commitlint fail in our project with Error: Cannot find module 'typescript' as of v17.7.1.~~ My mistake--this was caused by another dependency in our package, cz-conventional-changelog, requiring an earlier version of @commitlint/cli.

This is even more necessary given the rise of bun.

https://github.com/oven-sh/bun/issues/6184

Solved by #3722