Package can't be used by an "extended" tslint configuration

[apexskier]

Scenario and problem

I've got a custom tslint configuration that's intended to be shared among several projects. I want that custom configuration to include this package's rules. Since tslint's custom rules loading always uses a relative/absolute path, specifying "rulesDirectory": "node_modules/tslint-eslint-rules/dist/rules" breaks when my custom configuration is inside the node_modules of another package.

Say my custom config is the package tslint-config-custom. It's got tslint-eslint-rules as a dev and peer dependency (this style is copied from airbnb's linter setup and exposes a config like:

{
  "rulesDirectory": "node_modules/tslint-eslint-rules/dist/rules",
  "rules": {
    "no-constant-condition": true
  }
}

When I have another project with tslint-config-custom and tslint-eslint-rules as dev dependencies, this breaks:

$ tslint -c tslint.json --project tsconfig.json
Failed to load [path]/tslint.json: Could not find custom rule directory: [path]/node_modules/tslint-config-custom/node_modules/tslint-eslint-rules/dist/rules

Fix

To resolve this, I'd suggest allowing people to load these rules by extending this repository as a tslint configuration.

This means my tslint-config-custom looks like:

{
  "extends": [
    "tslint-eslint-rules"
  ],
  "rules": {
    "no-constant-condition": true
  }
}

I'll submit a PR shortly with the change.

[apexskier]

This matches what https://github.com/palantir/tslint-react does.

[jmlopez-rod]

Seems like the right move, considering that @jkillian wrote something similar when index.js was created: aa17d2e.

On the issue with a possible custom config, shouldn't we be using this project as a peerDependency? Even airbnb installs a custom configuration for eslint by declaring eslint and other packages as peers. See https://github.com/airbnb/javascript/tree/master/packages/eslint-config-airbnb. I looked more into it when deciding to look into publishing packages and I found this: npm/npm#11213

I'm not opposed to using extends as in the PR. It does make it easier to start using the rules without providing a configuration. I'm just not sure if this would open future pull request in which people will want to provide a set configuration for tslint-eslint-rules which will create problems outside the scope of the project.

The tslint-config-standard project seems to have solved the problem of path resolutions with this:

https://github.com/blakeembrey/tslint-config-standard/blob/master/tslint.js

@blakeembrey Any thoughts on this discussion? It would make your configuration much shorter.

[blakeembrey]

1. The change makes sense, would make it easier for people to use and I don't see any downside.
2. Probably makes sense to use peerDependency for the rules, but I know it can get messy with pre-releases because TSLint moves their "nightly" releases for TypeScript nightlies on a different schedule. This creates a really PITA approach where their nightly versions probably won't be valid with a peer dependency specified by this module and will result in NPM complaining. I'd recommending testing it for yourself first, but that's been my experience with TypeScript + TSLint - the versions make it tricky to do peer deps without NPM complaining heavily.

[apexskier]

Setting peerDependency's will be useful for ensuring users of this package install a valid version of typescript. (tslint is already a dependency, so that doesn't need to be a peer). I haven't experienced the issues @blakeembrey mentioned, but I also haven't worked on prereleases at all.

On the matter of recommending extends (as I've done) vs rulesDirectory (using tslint-config-standard's method):
I think extends is a much more understandable method, and it works for both json and js configs, where the path/require stuff only works for js.

@jmlopez-rod's point about opening the possibility for setting default rule settings in this package is valid, but I don't think it's a problem. If someone wants to add defaults to this package, they could be exposed in a separate configuration. (keep the package's main index.js unopinionated, and create a similar file like opinionated.js, allowing for extends: 'tslint-eslint-rules/opinionated')

[jmlopez-rod]

@apexskier we do have an opinionated configuration, that's the one we are use on our own source files https://github.com/buzinas/tslint-eslint-rules/blob/master/tslint_eslint_rules.json, we should probably add a comment on index.js to make it clear that the configuration needs to remain onopinionated and that it only exists to make it easier to access the rules in the project.